CODEWITHSUNDEEP
javascriptjqueryhtmlarraysrecursion
Adam
Adam

Reputation: 61

Recursively Calling a Function in Javascript?

I am trying to create a little piece of interactive fuction using HTML and jQuery. To do this I have been using jQuery to update and change the attr() and .text(). I wanted to do this approach because I wanted a more seamless feel without page loads since it is not image intensive.

I created "scenes" which have a basic title. These titles I put into an array and then shuffle them (I want their appearance to be random and not repeat).

Now the scenes only change when the user makes a choice using radio buttons (I may change this as currently the radio buttons do not clear when the content changes). So, one the click of the ID's for those choices, in jQuery I have the "random_scene" function and try to call this same function again; basically trying to make it recursively step through the array until all elements are used.

What I have below, works, kind of, but it is glitchy and sometimes skips a selections without or steps down through the whole array at once before landing on the last element.

I need help in determining if what I am attempting to do is even possible and if there is a better way to do what I am doing. I'll include my whole js file so you can hopefully see what's going on. 

$('#menu').accordion({  fillSpace: true, heightStyle: "content",  
event: 'mouseover', collapsible: true,  active: false, icons: false, animate: 750     });

$('.hover').mousemove(function(){
$(this).css('opacity', 1.0);
}).mouseout(function(){
    $(this).css('opacity', 0.5);    
}); 




 function rand_scene(scenes){

        if ( scenes[scenes.length-1] == "prison0" ){

            scenes.pop();


            $('img').attr('src', 'images/Prison.jpg');

            $('audio').attr('src', 'audio/');

            $('#choice1').text('choice content 1'); 

            $('#choice2').text('choice content 2');

            $('#choice3').text('choice content 3');

            $('#check1').click(function(){
                score += 2;
                return rand_scene(scenes);
            });

            $('#check2').click(function(){
                return rand_scene(scenes);
            });

            $('#check3').click(function(){
                score++;
                return rand_scene(scenes);
            });             
        }

        if ( scenes[scenes.length-1] == "socserv1" ){

            scenes.pop();

            $('img').attr('src', 'images/soc_serv.jpg');

            $('audio').attr('src', 'audio/');

            $('#choice1').text('choice content 1'); 

            $('#choice2').text('choice content 2');

            $('#choice3').text('choice content 3');

            $('#check1').click(function(){
                score += 3
                return rand_scene(scenes);      
            }); 

            $('#check2').click(function(){
                score++;
                return rand_scene(scenes);
            }); 

            $('#check3').click(function(){
                score += 2;
                return rand_scene(scenes);
            }); 

        }   

        if ( scenes[scenes.length-1] == "college3" ){

            scenes.pop();

            $('img').attr('src', 'images/college_debt.jpg');

            $('audio').attr('src', 'audio/');

            $('#choice1').text('choice content 1'); 

            $('#choice2').text('choice content 2');

            $('#choice3').text('choice content 3');

            $('#check1').click(function(){
                return rand_scene(scenes);      
            }); 

            $('#check2').click(function(){
                return rand_scene(scenes);
            }); 

            $('#check3').click(function(){
                return rand_scene(scenes);
            }); 

        }   

        if ( scenes[scenes.length-1] == "unity2" ){

            scenes.pop();

            $('img').attr('src', 'images/united.jpg');

            $('audio').attr('src', 'audio/');

            $('#choice1').text('choice content 1'); 

            $('#choice2').text('choice content 2');

            $('#choice3').text('choice content 3');

            $('#check1').click(function(){
                return rand_scene(scenes);          
            }); 

            $('#check2').click(function(){
                return rand_scene(scenes);
            }); 

            $('#check3').click(function(){
                return rand_scene(scenes);
            });     
        }   


}


var scenes = [ "prison0", "socserv1", "unity2", "college3" ];

function shuffle(array) {
    var counter = array.length;
    var temp = 0; 
    var index = 0;

    // While there are elements in the array
    while (counter > 0) {
        // Pick a random index
        index = Math.floor(Math.random() * counter);

        // Decrease counter by 1
        counter--;

        // And swap the last element with it
        temp = array[counter];
        array[counter] = array[index];
        array[index] = temp;
    }

    return array;
}

$(document).ready(function(){

shuffle(scenes);
//document.getElementById("menu_ops").innerHTML = scenes;

//var rand_page = Math.floor(Math.random() * 4);
var score = 0;
//var cont = 1;
rand_scene(scenes);

});

As you can see a scene is selected. Some attributes and text are changed within the #choice1/2/3. The user then selects their choice in with the #check1/2/3 which are the radio buttons in the html. On the click of the radio button their score (it's internal they don't see it) is updated and the next "scene" should be pulled from the array.

As stated above, it sort of works, but it tends to skip the "prison0" scene unless the "prison0" scene is the last element in the shuffled array.

I have to be missing something with calling the functions like this.

Thanks for any help and please let me know if there's anything else I need to clarify. Thank you.

Here is the fiddle link. It doesn't look great, but the selection radios at the bottom can be seen and some of the scenes do change. I did change to the else if statements.

jsfiddle

EDITADD: I figured the problem out on my own. In the code above I was popping from the array the moment the new scene was selected. In my own code I moved the pop() to within the $()click(function(){};

That solved everything and gave me the result I needed. Thanks all for the help!

Upvotes: 1

Views: 293

Answers (2)

isick
isick

Reputation: 1487

It would be helpful if you had this in a fiddle. But from what I gather, here's two improvements you can make (one of which is I think what you mean by recursion)

1) Your shuffle function isn't the best. Because index can only be in the range [0, counter] instead of [0, array.length] it will unfairly skew the shuffle towards the smaller indexed elements. Which means it's unlikely to get a real shuffle. What might better is to say,

index = Math.floor(Math.random() * scenes.length);

To ensure all elements have a (more) fair chance of getting shuffled.

2) This problem doesn't call for recursion. It can be accomplished with a simple loop. Where you say, rand_scene(scenes); is where I suppose you intend to initiate your recursive call, but this is unnecessary. Instead, you can simply place that call into a while loop and run for as long as there are elements in the array.

while(scenes.length){
    rand_scene(scenes);
}

And because rand_scene(scenes); is popping every time, we can be assured that at some point, scenes.length will be 0 and the loop will break.

Upvotes: 0

Reinstate Monica Cellio
Reinstate Monica Cellio

Reputation: 26143

The first thing I would suggest is changing the 2nd, 3rd & 4th if statements in the rand_scene function to else if, so they only run if the previous if statement wasn't satisfied.

This would fix the problem that you could essentially satisfy all if statements in that function. Take this example...

var ar = [1,2,3,4];

if (ar[ar.length - 1] == 4) {
    console.log("four");
    ar.pop();
}
if (ar[ar.length - 1] == 3) {
    console.log("three");
    ar.pop();
}
if (ar[ar.length - 1] == 2) {
    console.log("two");
    ar.pop();
}
if (ar[ar.length - 1] == 1) {
    console.log("one");
    ar.pop();
}

The first if statement will evaluate to true since the last element in the array is 4. This will log "four" and then remove the last element of the array.

Then, the next if statement will also evaluate to true since the last element of the array is now 3.

Etc..

If, however, you change it to this then it will stop at one true if statement, and not run all of them...

var ar = [1,2,3,4];

if (ar[ar.length - 1] == 4) {
    console.log("four");
    ar.pop();
}
else if (ar[ar.length - 1] == 3) {
    console.log("three");
    ar.pop();
}
else if (ar[ar.length - 1] == 2) {
    console.log("two");
    ar.pop();
}
else if (ar[ar.length - 1] == 1) {
    console.log("one");
    ar.pop();
}

Without knowing how much of your code was pseudo and how much was literal I can't help with further tidying up, but there's a lot of repetition there that could be removed, making the code a lot cleaner and easier to read and maintain.

This is obviously only an example, but it's the same as above but trimmed down...

var ar = [1,2,3,4];
var value = ar.pop();

switch(value) {
    case 1:
        console.log("one");
        break;

    case 2:
        console.log("two");
        break;

    case 3:
        console.log("three");
        break;

    case 4:
        console.log("four");
        break;
}

Upvotes: 1

Related Questions