-2

I am trying to implement a simple JS random quote generator using the html and JS code below. However, it is not returning any quotes. I am a newbie in JS. Appreciate any help on this.

HTML

<!DOCTYPE html>
<html>
<head></head>
<body>
<script src="https://abcd.com/wp-includes/js/quotes.js"></script>
<script>
showQuotation();
</script>
</body>
</html>

JS (This is the code in https://abcd.com/wp-includes/js/quotes.js)

<script type="text/javascript">
// Copyright 2004 by CodeLifter.com
var Quotation=new Array()
Quotation[0] = "Time is of the essence! Comb your hair.";
Quotation[1] = "Sanity is a golden apple with no shoelaces.";
Quotation[2] = "Repent! The end is coming, $9.95 at Amazon.";
Quotation[3] = "Honesty blurts where deception sneezes.";
Quotation[4] = "Pastry satisfies where art is unavailable.";
Quotation[5] = "Delete not, lest you, too, be deleted.";
Quotation[6] = "O! Youth! What a pain in the backside.";
Quotation[7] = "Wishes are like goldfish with propellors.";
Quotation[8] = "Love the river's \"beauty\", but live on a hill.";
Quotation[9] = "Invention is the mother of too many useless toys.";
var Q = Quotation.length;
var whichQuotation=Math.round(Math.random()*(Q-1));
function showQuotation(){document.write(Quotation[whichQuotation]);}
showQuotation();
</script>
Thomas Koipuram
  • 19
  • 5
  • 3
    A JS file should not contain the `` and `` tags, those are for embedding JS in HTML. – jcaron Mar 26 '17 at 16:22
  • 2
    When doing web development, it's essential to keep the dev tools open and look for errors in the web console, step through code in the built-in debugger, etc. – T.J. Crowder Mar 26 '17 at 16:22
  • 2
    Also note that you're showing the quotation twice, once in `quotes.js` (at the end), and again later with inline script. – T.J. Crowder Mar 26 '17 at 16:24
  • 1
    Also note your `var Quotation = new Array()` should be defined as `var Quotation = ["q1", "q2", ...]`. You can have each quotation on new lines to keep it a similar structure too. [Justification for not using `new Array()`](https://coderwall.com/p/h4xm0w/why-never-use-new-array-in-javascript) – Liam Gray Mar 26 '17 at 16:24
  • 1
    Use `Quotation[Math.floor(Math.random() * Quotation.length)]` for selecting a random quote. Otherwise the first and last element are each going to have half the probability of the other elements. Also, the variables `Q` and `whichQuotation` are pointless because they’re only used once. Also, [don’t use `document.write`](http://stackoverflow.com/q/802854/4642212). – Sebastian Simon Mar 26 '17 at 16:32
  • @jcaron Thanks. It worked. – Thomas Koipuram Mar 26 '17 at 16:33
  • @Crowder Totally agree. This was just a one time activity I needed so did not bother to install any dev tools. But I do understand its importance. Also thanks for pointing out about showing the quotation twice. Corrected. – Thomas Koipuram Mar 26 '17 at 16:36
  • @Liam Gray Thanks. Will do that. – Thomas Koipuram Mar 26 '17 at 16:39
  • @ThomasKoipuram You don’t need to install anything. They’re right in your browser. [Here’s how to open them](http://webmasters.stackexchange.com/q/8525) (simply hit `F12`). – Sebastian Simon Mar 26 '17 at 16:39
  • @Xufox Thanks for the comment regarding the random logic. I'll change the code accordingly. – Thomas Koipuram Mar 26 '17 at 16:40
  • @Xufox. Thanks! – Thomas Koipuram Mar 26 '17 at 16:50

1 Answers1

0

As per comments provided by all, I have modified my code as below. Let me know if its technically correct. From a requirement perspective, it is working as expected. Not sure if its the right way to convey it here (by posting an answer). If not, please let me know.

HTML

<p id="quotes-div"></p>
<script src="https://abcd.com/wp-includes/js/quotes.js">
</script>

JS Code inside quotes.js

// Copyright 2004 by CodeLifter.com
var Quotation=
[
    "Time is of the essence! Comb your hair.",
    "Sanity is a golden apple with no shoelaces.",
    "Repent! The end is coming, $9.95 at Amazon.",
    "Honesty blurts where deception sneezes.",
    "Pastry satisfies where art is unavailable.",
    "Delete not, lest you, too, be deleted.",
    "O! Youth! What a pain in the backside.",
    "Wishes are like goldfish with propellors.",
    "Love the river's \"beauty\", but live on a hill.",
    "Invention is the mother of too many useless toys.",
];
function showQuotation()
{
    document.getElementById("quotes-div").innerHTML = Quotation[Math.floor(Math.random() * Quotation.length)]
}
showQuotation();
jcaron
  • 16,007
  • 5
  • 28
  • 43
Thomas Koipuram
  • 19
  • 5
  • I have edited your answer to change the indentation of the Quotation definition, it should make things easier to read. – jcaron Mar 27 '17 at 08:28