tytbone Posted July 6, 2014 Share Posted July 6, 2014 I decided to try to make a random phrase generator based around the "Looks like I picked the wrong week..." quote from Airplane! (I'm just testing it with one word at first.) I'm trying to ensure that the next phrase will be different than the last, and I thought I could do that by storing the index item in a seperate var and comparing that var to the current index item the next time the button is pressed. Currently however I cannot get any text to generate. I'm assuming there's something wrong with my logic or I missed something really obvious; help would be appreciated. <body> <p>Looks like I chose the wrong week to quit <span id="generate"></span></p> <button onclick="fromLists()">Change</button> <script> var verbs = ["kicking.", "chewing.", "annoying.", "hurting.", "breathing."]; var verbCurr; var verbPrev; function fromLists() { for (i = 0; i<verbs.length; ++i) { verbCurr = verbs[Math.round(Math.random()*verbs.length)]; if (verbCurr === verbPrev) { verbCurr = verbs[Math.round(Math.random()*verbs.length)]; } else { document.getElementById("generate").innerHTML = verbCurr; verbPrev = verbCurr; } } </script> </body> http://jsfiddle.net/gogfan/CbLXC/8/ Link to comment Share on other sites More sharing options...
dsonesuk Posted July 6, 2014 Share Posted July 6, 2014 count '{' compared to '}' 1 Link to comment Share on other sites More sharing options...
tytbone Posted July 6, 2014 Author Share Posted July 6, 2014 count '{' compared to '}' Ah, thank you, I missed a bracket. Unfortunately the generator still isn't working how I would like it to. I'm getting an occasional "undefined" and it appears sometimes the same word will appear twice in a row, as sometimes the word won't change even when the button is pressed. Any idea what changes should be made to fix this? Do I need to implement the "i" from the for loop into the code? Should there be more than one function, and if so should I implement a "return" statement to help with comparing? Link to comment Share on other sites More sharing options...
Hadien Posted July 6, 2014 Share Posted July 6, 2014 Why are you using a for loop? the function should only need to grab the word once. I've made some other changes as well, should help fix some other issues. <body> <p>Looks like I chose the wrong week to quit <span id="generate"></span></p> <button onclick="fromLists()">Change</button> <script> var verbs = ["kicking.", "chewing.", "annoying.", "hurting.", "breathing."]; function fromLists() { //verbCurr is not needed outside the function // the length of verbs is 5, but should only range from 0 to 4 var verbCurr = verbs[Math.round(Math.random()*(verbs.length-1))]; // verbPrev is also not needed we can simply use the node itself var span = document.getElementById("generate"); if (verbCurr === span.innerHTML ) { //if the current and span words match just redo the function again return fromLists(); } span.innerHTML = verbCurr; } </script> </body> 1 Link to comment Share on other sites More sharing options...
tytbone Posted July 6, 2014 Author Share Posted July 6, 2014 Why are you using a for loop? the function should only need to grab the word once. I've made some other changes as well, should help fix some other issues. <body> <p>Looks like I chose the wrong week to quit <span id="generate"></span></p> <button onclick="fromLists()">Change</button> <script> var verbs = ["kicking.", "chewing.", "annoying.", "hurting.", "breathing."]; function fromLists() { //verbCurr is not needed outside the function // the length of verbs is 5, but should only range from 0 to 4 var verbCurr = verbs[Math.round(Math.random()*(verbs.length-1))]; // verbPrev is also not needed we can simply use the node itself var span = document.getElementById("generate"); if (verbCurr === span.innerHTML ) { //if the current and span words match just redo the function again return fromLists(); } span.innerHTML = verbCurr; } </script> </body> Thanks, as I wrote in the post I'm a newbie, and have difficulties understanding programming in general. I assumed a for loop would be needed to search through all the index items. So length doesn't start at 0, I presume, which is why the -1 is needed. And "return" has the ability to "restart" the function if necessary? "Return" to a specific point. I'll test the code out in a few hours; again, thanks. Link to comment Share on other sites More sharing options...
Hadien Posted July 6, 2014 Share Posted July 6, 2014 Index does start at 0. it moves through like [0, 1, 2, 3, 4]. Verbs.length will return the number 5 (since there are five elements). while Math.random() generates a random floating number from 0 to 0.99999 which you then multiply by the length to get a number ranging 0 to 4.99999. Using Math.round() can round the number upto 5 which would cause an out of bounds error. I simply subtracted the length by 1 to avoid that. you can just as well use Math.floor() instead of Math.round() which will always round the floating number down so you don't have to worry about subtracting the verbs.length by 1. 1 Link to comment Share on other sites More sharing options...
tytbone Posted July 6, 2014 Author Share Posted July 6, 2014 Index does start at 0. it moves through like [0, 1, 2, 3, 4]. Verbs.length will return the number 5 (since there are five elements). while Math.random() generates a random floating number from 0 to 0.99999 which you then multiply by the length to get a number ranging 0 to 4.99999. Using Math.round() can round the number upto 5 which would cause an out of bounds error. I simply subtracted the length by 1 to avoid that. you can just as well use Math.floor() instead of Math.round() which will always round the floating number down so you don't have to worry about subtracting the verbs.length by 1. Thanks, I didn't think about rounding up to 5. I may use Math.floor like you suggested. Link to comment Share on other sites More sharing options...
tytbone Posted July 6, 2014 Author Share Posted July 6, 2014 I like what I have and it seems to work, but I'm wondering if there's a better way to handle this. I have two arrays, one for the verbs and one for the nouns, and they are being handled seperately in the same function (basically a copy-paste of Hadien provided). Is there a better way to handle this? I felt like the verb and the noun needed to be handled and checked seperately which is why they have their own id's, but maybe there's a way to simplify. Thanks preemptively. <body> <p>Looks like I chose the wrong week to quit <span id="verb"></span> <span id="noun"></span>.</p> <button onclick="fromLists()">Change</button> <script> var verbs = ["kicking", "chewing", "annoying", "hurting", "breathing"]; var nouns = ["babies", "Steve Erkel", "Norway", "linguine", "toothpaste"]; function fromLists() { var verb = verbs[Math.floor(Math.random()*verbs.length)]; var noun = nouns[Math.floor(Math.random()*nouns.length)]; var spanVerb = document.getElementById("verb"); var spanNoun = document.getElementById("noun"); if (verb === spanVerb.innerHTML || noun === spanNoun.innerHTML) { return fromLists(); } spanVerb.innerHTML = verb; spanNoun.innerHTML = noun; } </script></body> Link to comment Share on other sites More sharing options...
Recommended Posts
Create an account or sign in to comment
You need to be a member in order to leave a comment
Create an account
Sign up for a new account in our community. It's easy!
Register a new accountSign in
Already have an account? Sign in here.
Sign In Now