Random quote rotator

Question

On my website, I created like this banner that rotates quotes from an array I made.

var myarray = []
myarray[0] = "\"To a contrarian like me, constant advice not to do something almost always starts me quickly down the risky, unpopular path.\" <br> <strong> Micheal Bloomberg.</strong>"; 
myarray[1] = "\"In investing, what is comfortable is rarely profitable.\" <br><strong> Robert Arnott</strong>"; 
myarray[2] = "\"Know what you own, and know why you own it.\" <br><strong> Peter Lynch</strong>";
myarray[3] = "\"Buy when everyone else is selling and hold until everyone else is buying. That’s not just a catchy slogan. It’s the very essence of successful investing.\"  <br><strong> J. Paul Getty</strong>";
myarray[4] = "\"When something is important enough, you do it even if the odds are not in your favor.\" <br><strong> Elon Musk</strong>";
myarray[5] = "\"I will tell you the secret to getting rich on Wall Street. You try to be greedy when others are fearful. And you try to be fearful when others are greedy.\" <br><strong> Warren Buffet</strong>";
myarray[6] = "\"Innovation distinguishes between a leader and a follower.\" <br> <strong>Steve Jobs</strong>";
myarray[7] = "\"The only thing that makes sense is to strive for greater collective enlightenment.\" <br><strong> Elon Musk</strong>"
myarray[8] = "\"The single best piece of advice: Constantly think about how you could be doing things better and questioning yourself.\"<br><strong> Elon Musk </strong>"
//Quote randomizer I made.

function quote() {
    var x = Math.floor(Math.random() * (myarray.length));
    document.getElementById("quotes").innerHTML = myarray[x];

}

var myVar = setInterval(quote, 15000);

Is there a better way to write this code? I wrote it back in February of this year. Now, I am just curious if there is any way to improve it.

For those who want to see it in action, scroll down past the "meet the team" section after you click here.

Answer 1

Cache DOM selectors

You select the target container over and over again. You should get it once outside the function, cache it in a variable and reuse it:

const target = document.getElementById('quotes');

Naming

Try to use descriptive names instead of myarray, myVar and quote:

const quotes = [];
function changeQuoteRandomly() {}

Style

It's easier to read, if you use single quotes on the strings, as you don't have to escape the double quotes:

quotes.push('"Important words" by Someone');

You sometimes use a semicolon at the end of the line and sometimes you don't – use only one way and be consistent:

var myarray = []
var myVar = setInterval(quote, 15000);

I would prefer using push to add elements to the array instead of setting the index manually. Or you could even add all elements during initialization:

const quotes = [
    '"Important words" by Someone',
    '"Other important words" by Someone'
];

Both ways make it easier to re-arrange elements later as you don't have to keep track of the index yourself.

Semantics

Currently your markup looks like this:

<div id="quotes">
    "The single best piece of advice: Constantly think about how you could be doing things better and questioning yourself."<br>
    <strong> Elon Musk</strong>
</div>

You could improve this, by using blockquote, cite and even figure elements. Here you can find more infos and examples on W3C: 4.4.5. The blockquote element.

User experience

While this is a nice addition on your website, there are a few downsides:

• There's no introduction to this section, the visitor has no clue why you're showing them. Try to relate them to your business.
• The first quote is shown after 15s, until then only an empty blue container ist visible.
• 15s between quotes is a really long time. There's no indicator that more is coming, so there's a great chance that a user will not see a second one.

Answer 2

Text use textContent

When assign content to an element use textContent rather than innerHTML This prevent a page reflow and all the other overheads associated with new elements.

setTimeout for better control

Use setTimeout rather than setInterval. It lets you start immediately and it also lets you control the display time based on the content. eg use word count to set display time.

For example

const interval = 500; // in ms per word
myTimedFunction();      // starts the timed function immediately.
function myTimedFunction(){

    // do something
    var wordCount = getWordCount(quote); // imagined function 

    setTimeout(myTimedFunction, interval * wordCount);
}

Content where it belongs

First... You don't need to manually allocate items to an array.

You had something like

const arr = [];
arr[0] = "blah";
arr[1] = "blah";
arr[2] = "de blah";
arr[3] = "blah";

Which can be done as

const arr = [;
    "blah",
    "blah",
    "de blah",
    "blah",
];

However the quotes would be better stored hidden on the page, as it is content not data.

<!-- HTML -->
<ul class="quotes--quotes-hidden">
    <li>Blah</li>
    <li>Blah</li>
    <li>De Blah</li>
    <li>Blah</li>
</ul>

/*CSS*/
.quotes--quotes-hidden { display : none; }

And then you can extract it and add it randomly to the display element.

const interval = 15000;
const displayEl = document.getElementById("quotes");
const quotes = [...document.querySelectior(".quotes--quotes-hidden").children];
randomQuote();
function randomQuote() {
    // NOTE (val | 0) is the same as Math.floor(val)
    displayEl.textContent = quotes[Math.random() * quotes.length | 0].textContent;
    setTimeout(randomQuote, interval);

}

Don't repeat random items.

• Random can seam very none random to humans. For a small list of items the odds of the same item being repeated is 1/n where n is the number of items. So for 10 items, on average 1 in every ten users will have to wait 30 seconds before they see an new quote, 1 in 100 will wait 45, and 1 in a 1000 will wait 60. Also 1 in ten will see the first quote repeated after the second quote has been displayed.

  To avoid repeating the same content use a second array to store upcoming quotes. Each time one is displayed remove it from the array. When the array is empty fill it again and then you start the repeat.

For example

const interval = 15000;
const displayEl = document.getElementById("quotes");
const quotes = [...document.querySelectior(".quotes--quotes-hidden").children];
const quotesPending = [];
randomQuote();
function randomQuote() {
    if (quotesPending.length === 0) { // get next round
       quotesPending.push(...quotes);
    } 
    const next = quotes.splice(Math.random() * quotes.length | 0)[0];
    displayEl.textContent = next.textContent;
    setTimeout(randomQuote, interval);

}

This will still let in the occasional repeat. To stop repeats you randomly shuffle the array then random remove one of the first half, display it and push it to the end of the array. This will ensure that a min of n/2 (where n is number items) quotes will be displayed before there is a repeat.

Here are three utility functions for no repeat random selection of a fixed set of items. As compact form, I can not be bothered expanding them and would be used as follows.

// quotes is the array of random quotes

// do once at start
randShuffle(quotes); // randomizes the array

// need mid point 
const quotedSizeHalf = quotes.length / 2;

// Do each time you want a new quote
const noRepeatRandQuote = randPlay(quotes, quotedSizeHalf);

The trade of is that you slightly increase the chance of a sequence. The distribution remains flat

// The required functions.
const randI = r => Math.random() * r | 0;
const randShuffle = (a, l = a.length) => { while (l) { a.push(a.splice(randI(l--), 1)[0]) } return a }
const randPlay = (a, r, i = a.splice(randI(r), 1)[0]) => (a.push(i), i);