Welcome to the Treehouse Community

Want to collaborate on code errors? Have bugs you need feedback on? Looking for an extra set of eyes on your latest project? Get support with fellow developers, designers, and programmers of all backgrounds and skill levels here with the Treehouse Community!

Looking to learn something new?

Treehouse offers a seven day free trial for new students. Get access to thousands of hours of content and join thousands of Treehouse students and alumni in the community today.

Start your free trial

JavaScript

Bouke Kneepkens
seal-mask
.a{fill-rule:evenodd;}techdegree
Bouke Kneepkens
Full Stack JavaScript Techdegree Student 3,354 Points
Posted by Bouke Kneepkens
.a{fill-rule:evenodd;}techdegree
Bouke Kneepkens
Full Stack JavaScript Techdegree Student 3,354 Points

Trying not to duplicate a quote, quote generator

I wrote the code below. It makes sense to me... but unfortunately doesn't work :-/ Could someone please give me some feedback?

function getRandomQuote() {

var randomNumber = Math.floor(Math.random() * quotes.length);
var randomQuote = quotes[randomNumber];

//I added the code below to not duplicate a quote before all quotes have been shown once.

var usedQuotes = [];
usedQuotes.push('randomNumber');
for (var j = 0; j<usedQuotes.length; j++) {
  if (randomNumber === usedQuotes[j]){
   randomNumber;
 } else if  {
   return randomQuote;
 }
 }

// When all have been shown once start over if (usedQuotes.lenght === quotes.lenght){ usedQuotes = " "; } }

Gabriel Plackey
Gabriel Plackey
11,064 Points
Gabriel Plackey
Gabriel Plackey
11,064 Points

When you run this code, what does the console say?

3 Answers

Steven Parker
Steven Parker
225,756 Points
Steven Parker
Steven Parker
225,756 Points

Don't put quotes around "randomNumber", that pushes the name itself instead of the number it represents. Also, it looks like you have "lenght" instead of "length" in two places.

And by itself, "randomNumber;" as a statement doesn't do anything. Did you mean to pick another number there?

You might also want to consider a different strategy. The chances of picking a duplicate will increase as more picks are mode, so the program will spend more time re-picking. Another way to do this would be to take the picked items away from the main list and save them in a "used" list. Then each pick will be guaranteed to be new, until you use them all. At that point, you move the used list back into the main list and start over.

Bouke Kneepkens
seal-mask
.a{fill-rule:evenodd;}techdegree
Bouke Kneepkens
Full Stack JavaScript Techdegree Student 3,354 Points
Bouke Kneepkens
.a{fill-rule:evenodd;}techdegree
Bouke Kneepkens
Full Stack JavaScript Techdegree Student 3,354 Points

Sorry, I meant to write the message below in an answer to your message:

Thank you Steven! Your advice pushed me in the right direction. I'm now still struggling with how to use the slice method. The console shows: script.js:34 Uncaught TypeError: usedRandomQuotes.splice is not a function

It's for this piece of code:

if (quotes = " ") {
  quotes = usedRandomQuotes.splice(0, usedRandomQuotes.length);
  getRandomQuote();
}
return randomQuote;

Here's a GitHub link to the full project: boukekneepkens/a_random_quote_generator-v1 Would be great if you could help me understand how to get this working.

Steven Parker
Steven Parker
225,756 Points
Steven Parker
Steven Parker
225,756 Points

That link isn't quite complete, but the slice code to reset the array seems fine. However, instead of replacing the array with a space, you probably meant to test if it is empty. This would work better:

if (quotes.length == 0) {
Bouke Kneepkens
seal-mask
.a{fill-rule:evenodd;}techdegree
Bouke Kneepkens
Full Stack JavaScript Techdegree Student 3,354 Points
Bouke Kneepkens
.a{fill-rule:evenodd;}techdegree
Bouke Kneepkens
Full Stack JavaScript Techdegree Student 3,354 Points

Thank you Steven for your quick reply. Sorry for the incomplete link, beginners mistake: https://github.com/boukekneepkens/a_random_quote_generator-v1 I've added some comments where in the code things stop working (line 34 of the js/script.js). Can you explain to me what goes wrong there?

Steven Parker
Steven Parker
225,756 Points
Steven Parker
Steven Parker
225,756 Points

I see three issues:

  • "usedRandomQuotes" should be initialized just one time, outside of the function
  • "splice" returns an array, but you want a single quote
  • "push" adds to the array and returns the new length, you don't want to reassign the array with it
var usedRandomQuotes = [];                              // do this outside of function
// ...
  var randomQuote = quotes.splice(randomNumber, 1)[0];  // get one quote object
  usedRandomQuotes.push(randomQuote);                   // don't reassign array
Bouke Kneepkens
seal-mask
.a{fill-rule:evenodd;}techdegree
Bouke Kneepkens
Full Stack JavaScript Techdegree Student 3,354 Points
Bouke Kneepkens
.a{fill-rule:evenodd;}techdegree
Bouke Kneepkens
Full Stack JavaScript Techdegree Student 3,354 Points

Thanks Steven,

Thanks for your help! It's highly appreciated It works just fine now. I do have to admit that I still don't understand what the [0] does in the splice method. Neither can I find anything about it when I googled it. Do you maybe have a link with more info about this? I do see that an array is returned. I couldn't find why adding [0] would return the object inside :-/

I do understand the other two issues. UsedRandomQuotes = [] needs to be called outside of the function because else it can't receive anything from outside the scope and the quotes are in the global scope. Great, this makes sence!

Steven Parker
Steven Parker
225,756 Points
Steven Parker
Steven Parker
225,756 Points

Brackets are the indexing operator. So if you have an array (or a function that returns an array), the brackets select a single item from it based on the index. And an index of 0 is the first item (and in this case, it's the only item).