Random Quote Feedback

I do not know or have anyone to offer feedback or validation to any of my web design attempts, so…
My main concern is code clarity and efficiency, but any other tips would be helpful.

project link https://codepen.io/bakajan/pen/dMgwEr

Hi there,

Your design choice to offer different quote categories is really cool. I do have some feedback for your code, though they’re just nitpicky and (hopefully) informational.

First, you’ve got an initialization function (load), which is great, but it’s odd to have it on the body tag’s onload event. As far as I know, there’s no performance penalties or anything technically wrong with this, but it means that in order to understand the flow of your code, I have to flip between your html and js file. It’s not a big deal here, but in larger projects that can make reading your code much harder. The same can be said for the other event handlers. The more typical choice is to use jQuery’s ready event on the document.

$(document).ready(function(){

/* code here */
$('#some-element').click(eventHandler);

load() //Run the load function for the whole program

}

Your quotes are organized in an object by their type, but you’ve got each quote and its author in a 2-dimensional array. Later on, you reference these by random[0] and random[1]. That certainly works, but imagine you want to add some information to these quotes, like a date. It could get really difficult to look at the quotes themselves if they’re just sitting in arrays like that. An alternative is to use make each quote an object, and keep the quote objects in arrays.

{
    "hope": [
            {
                "author": "H. Jackson Brown, Jr.",
                "quote": "The best preparation for tomorrow is doing your best today."
            },
            {
                "author": "Jimmy Dean",
                "quote": "I can't change the direction of the wind, but I can adjust my sails to always reach my destination."
            }
        ],
    "love": [
        /* and so on */
    ]
}

Now, you can still get a random number to access an array of quotes, like love[1], but reference the name and quote more descriptively.

random = Math.floor(Math.random() * quotes.hope.length);
selectedQuote = quotes.hope[random].quote;
selectedAuthor = quotes.hope[random].name;

Lastly, and I’ll leave it up to you to figure out how exactly to do this, what if instead of relying on if/else clauses, you had separate functions for getting each type of quote? I think that would be easier to read, but would it be worth it to write? What about if your boss wanted another 5 categories for quotes?

I hope any or all of this helped. I wouldn’t necessarily say that you need to change anything at all, but based on what I read in your code, these would be great questions to have at the front of your mind when working on your next project. Overall, I think it’s great and you engineered workable solutions and got a nifty app without too much code. Well done!

1 Like

Don’t be so hard on yourself. You should be getting it to work before you clean up the code. Definitely stick with that. Premature optimization is the bad habit. The trick is just to learn how to clean up the code once it works.

1 Like