Displaying random quotes from an array

Question 1

Is there anything that I could have done better in this code?

<!DOCTYPE html>
<html>
<head>
<meta charset="utf-8">
<title>Quotes</title>
<style>
body { color: #333; font: 20px georgia; }
em { color: #555; font-size: 90%; }
</style>
</head>
<body>
<article id="quotes"></article>
<script>
(function() {
 "use strict";
 var quotes = [
 ["Stay Hungry. Stay Foolish.", "Steve Jobs"],
 ["Good Artists Copy, Great Artists Steal.", "Pablo Picasso"],
 ["Argue with idiots, and you become an idiot.", "Paul Graham"],
 ["Be yourself; everyone else is already taken.", "Oscar Wilde"],
 ["Simplicity is the ultimate sophistication.", "Leonardo Da Vinci"]
 ].sort(function() {
 return 0.5 - Math.random();
 }),
 quotesHTML = "";
 for (var i = 0; i < quotes.length; i++) {
 quotesHTML += "<p>&ldquo;" + quotes[i][0] + "&rdquo; &mdash; <em>" + quotes[i][1] + "</em></p>";
 }
 document.getElementById("quotes").innerHTML = quotesHTML;
}());
</script>
</body>
</html>

Question 2

I think your code looks pretty good already. It follows conventions and your indentation and variable names are precise. There's not much to review.

But if you're targeting modern browsers (IE9+) I would suggest a different approach using map and would separate the random logic into a function. I'd also declare the element up top so I know at first glance that the code is working with the DOM. It may seem like more code at first but it's good for code reuse, plus I think it reads better; the intent is more clear:

(function() {
 "use strict";
 var el = document.getElementById("quotes");
 var quotes = [
 ["Stay Hungry. Stay Foolish.", "Steve Jobs"],
 ["Good Artists Copy, Great Artists Steal.", "Pablo Picasso"],
 ["Argue with idiots, and you become an idiot.", "Paul Graham"],
 ["Be yourself; everyone else is already taken.", "Oscar Wilde"],
 ["Simplicity is the ultimate sophistication.", "Leonardo Da Vinci"]
 ];
 function rand(xs) {
 return xs.slice(0).sort(function(){
 return .5 - Math.random();
 });
 }
 function quote(q) {
 return "<p>&ldquo;"+ q[0] +"&rdquo; &mdash; <em>"+ q[1] +"</em></p>";
 }
 el.innerHTML = rand(quotes).map(quote).join('');
}());

Question 3

Based upon Jiving's answer and his markup, I'd suggest moving the quote signs to the CSS. For this purpose I added p tags inside blockquote. I also changed the author class to source.

"<blockquote class="random-quotes"><p>"+ q.quote +"</p><footer class="source"> &mdash;"+ q.author +"</footer></blockquote>"

(/!\ Note: Since the p tags are hard coded into my example, this would only work with single paragraph quotes. I don't know if this is enough. If not, you'll need to pass the p tags with the quotes.)

CSS:

blockquote p:first-of-type:before {
 content: "201円C";
}
/* Selecting only the last paragraph for when there are multiple paragraphs */
blockquote p:last-of-type:after {
 content: "201円D";
}
blockquote p:last-of-type {
 margin-bottom: 0;
}
.source {
 display: block;
}

Question 4

Why :last-of-type when there's only one p?

Question 5

@Jivings That's for the edge case when there are multiple paragraphs. One usually wants the margin between the paragraphs, but not between paragraph and source. Also one only needs the quotation marks on the last paragraph. Ah, wait a sec... gonna need a :first-of-type for the open quote as well then. ;)

Question 6

That makes more sense :)

Question 7

The only thing I'd change is your data structure. I think it would work better as an array of objects, then you can access the quote and author without indexing an array.

var quotes = [
 { 
 quote: "Stay Hungry. Stay Foolish.", 
 author: "Steve Jobs"
 },
 [...]
];
[...]
"<blockquote>&ldquo;"+ q.quote +"&rdquo; <footer class="author"> &mdash;" 
 + q.author +"</footer></blockquote>"

Also, maybe use a <blockquote> element instead of a <p> for the correct semantics.

Question 8

The source of a quotation should be wrapped with a footer element inside the blockquote. Also there should be no space between the em dash and the author.

Question 9

@kleinfreund Done.

Question 10

Plus what I was trying to say: author is inappropriate here. It's not an author, but the source of the quote.

Question 11

@kleinfreund What's the difference?

Question 12

Tried googling that, but apparantly there isn't even an author element.^^

elclanrs elclanrs 2,55516 silver badges16 bronze badges · Answer 1 · 2014-02-09 05:24:25Z

I think your code looks pretty good already. It follows conventions and your indentation and variable names are precise. There's not much to review.

But if you're targeting modern browsers (IE9+) I would suggest a different approach using map and would separate the random logic into a function. I'd also declare the element up top so I know at first glance that the code is working with the DOM. It may seem like more code at first but it's good for code reuse, plus I think it reads better; the intent is more clear:

(function() {
 "use strict";
 var el = document.getElementById("quotes");
 var quotes = [
 ["Stay Hungry. Stay Foolish.", "Steve Jobs"],
 ["Good Artists Copy, Great Artists Steal.", "Pablo Picasso"],
 ["Argue with idiots, and you become an idiot.", "Paul Graham"],
 ["Be yourself; everyone else is already taken.", "Oscar Wilde"],
 ["Simplicity is the ultimate sophistication.", "Leonardo Da Vinci"]
 ];
 function rand(xs) {
 return xs.slice(0).sort(function(){
 return .5 - Math.random();
 });
 }
 function quote(q) {
 return "<p>&ldquo;"+ q[0] +"&rdquo; &mdash; <em>"+ q[1] +"</em></p>";
 }
 el.innerHTML = rand(quotes).map(quote).join('');
}());

user35408user35408 · Answer 2 · 2014-02-09 16:49:11Z

Based upon Jiving's answer and his markup, I'd suggest moving the quote signs to the CSS. For this purpose I added p tags inside blockquote. I also changed the author class to source.

"<blockquote class="random-quotes"><p>"+ q.quote +"</p><footer class="source"> &mdash;"+ q.author +"</footer></blockquote>"

(/!\ Note: Since the p tags are hard coded into my example, this would only work with single paragraph quotes. I don't know if this is enough. If not, you'll need to pass the p tags with the quotes.)

CSS:

blockquote p:first-of-type:before {
 content: "201円C";
}
/* Selecting only the last paragraph for when there are multiple paragraphs */
blockquote p:last-of-type:after {
 content: "201円D";
}
blockquote p:last-of-type {
 margin-bottom: 0;
}
.source {
 display: block;
}

@Jivings That's for the edge case when there are multiple paragraphs. One usually wants the margin between the paragraphs, but not between paragraph and source. Also one only needs the quotation marks on the last paragraph. Ah, wait a sec... gonna need a :first-of-type for the open quote as well then. ;)

Jivings Jivings 2,34114 silver badges17 bronze badges · Answer 3 · 2014-02-09 14:37:47Z

7

\$\begingroup\$

The only thing I'd change is your data structure. I think it would work better as an array of objects, then you can access the quote and author without indexing an array.

var quotes = [
 { 
 quote: "Stay Hungry. Stay Foolish.", 
 author: "Steve Jobs"
 },
 [...]
];
[...]
"<blockquote>&ldquo;"+ q.quote +"&rdquo; <footer class="author"> &mdash;" 
 + q.author +"</footer></blockquote>"

Also, maybe use a <blockquote> element instead of a <p> for the correct semantics.

Share

edited Feb 9, 2014 at 15:32

answered Feb 9, 2014 at 14:37

Jivings's user avatar

Jivings Jivings

2,34114 silver badges17 bronze badges

\$\endgroup\$

6

\$\begingroup\$ The source of a quotation should be wrapped with a footer element inside the blockquote. Also there should be no space between the em dash and the author. \$\endgroup\$

user35408
– user35408

2014年02月09日 14:50:53 +00:00
Commented Feb 9, 2014 at 14:50
\$\begingroup\$ @kleinfreund Done. \$\endgroup\$

Jivings
– Jivings

2014年02月09日 15:18:44 +00:00
Commented Feb 9, 2014 at 15:18
\$\begingroup\$ Plus what I was trying to say: author is inappropriate here. It's not an author, but the source of the quote. \$\endgroup\$

user35408
– user35408

2014年02月09日 15:27:16 +00:00
Commented Feb 9, 2014 at 15:27
\$\begingroup\$ @kleinfreund What's the difference? \$\endgroup\$

Jivings
– Jivings

2014年02月09日 15:28:00 +00:00
Commented Feb 9, 2014 at 15:28
\$\begingroup\$ Tried googling that, but apparantly there isn't even an author element.^^ \$\endgroup\$

user35408
– user35408

2014年02月09日 15:31:02 +00:00
Commented Feb 9, 2014 at 15:31

| Show 1 more comment

Stack Exchange Network

Displaying random quotes from an array

3 Answers 3

Your Answer

Sign up or log in

Post as a guest

Post as a guest

Hot Network Questions

Displaying random quotes from an array

3 Answers 3

Your Answer

Sign up or log in

Post as a guest

Post as a guest

Related

Hot Network Questions