Random Quote Machine -- review

Hey there,
My random quote machine, made with jQuery and sass with BEM syntaxe for the CSS/HTML, but I don’t like my loop recursive

1 Like

Which part don’t you like, and where is your recursive loop? Your project looks fine, though I admit I’m having a hard time understanding how your code works.

I cleaned my code and add coments, can you take a look?

EDIT:
my loop here:

  // loop
  //the first loop is to get quotes[0]
  $.each(quotes, function(key, val) {
    // I get a quote with a rand number
    $.each(quotes[randQuote],function (citation,author ) {
      authorTemp=author;
      citationTemp=citation;
    });
  });

I don’t understand, when I’m triing to got the first quote the is son, it doesn’t work

$.each(quotes[0][randQuote]

Thanks

Ah, I see. What you tried didn’t work because your syntax is off. It should be quotes[randQuote]. But there are some changes you can make to simplify this greatly. Here’s what I would do:

  1. Change the way you’re storing quotes so that your objects in the array have two properties, text and author.
var quotes = [{
    text: "If debugging is the process of removing software bugs, then programming must be the process of putting them in.",
    author: "Edsger Dijkstra (Dutch computer scientist, winner of the 1972 Turing Award)"
},
...
]}
  1. Use a random number to access an object in the array
var randomNumber = getRandomInt(0,quotes.length); //We shouldn't hardcode the upper limit
var randomQuote = quotes[randomNumber];
  1. Fill out your markup with the data
$('.box__quote').html(randomQuote.text).fadeIn(400,function () {
    $('.box__author').html(randomQuote.author).delay(500).fadeIn(400);
});

You don’t need to use a loop at all. As a side note, what you’re doing with the loops isn’t recursion, it’s nesting. Recursion is when a function calls itself:

function recursiveFunction(someNumber) {
   if(someNumber > 5) {
     return "I'm all done!";
   }
   recursiveFunction(someNumber++);
}

recursiveFunction(0);

Nesting is when you have something like a loop inside of a loop:

for(var i = 0; i < 5; i++) {
  for(var j = 0; j < 5; j++) {
   //...
   }
}

Hope this helps!

Well, thank you for your time, it’s more simple at all and much looks better to read too.
I will keep my loop because it works and I did the final job, but at the same time, I rather prefer your idea than my version.

For the recursive thank you for making clearly the two things.

I don’t know if I must re-write the challenge on your way, I will feel to just copy your solution.

I prefer your version++ is much smarter :slight_smile:

EDIT: I changed the code to your path.

EDIT2: I didn’t know I can call an object like the dom with jquery.
randomQuote.text

EDIT3
Can you look the code again?

EDIT4
finally, I changed everything to follow your recommendation:
I just add a function with a boolean parameter.

Why do I use an array and not an object and finally is not a JSON format but is not better to work with JSON and for primary data like our case?

var q = [
  { 
    text: 'dddd',
    author: 'dsfsdf sdf'
  },
  { 
    text: 'dddd',
    author: 'dsfsdf sdf'
  }
];
an object
var qJson ={
   {'ddsdf' : 'dddd'},
   {'ddsdf' : 'dddd'}
};

Thanks a lot.

JSON stands for JavaScript Object Notation. It’s really just a way of organizing data in the same way JavaScript objects are written (there are minor differences we can ignore). The reason we want to make objects the way I showed you is that we want to be able to get data from quote.text and quote.author. We want those object keys to be the same for every single quote.

We put them in an array because we want to get them with a random number, and we can’t do that with objects.

1 Like

make sens I redo my project with your advice and is much better, thanks again.