Switching font and background color on button click

Question

I am currently building a web app where I need to randomly switch the font and color of a title (inside the #content div) and the background of the page every time a button is clicked.

I wrote the following code that works perfectly, but I'm wondering if it could be optimized (or at least written in a more elegant way).

// Switch font randomly
var font = ["lobster", "shadows", "oswald", "josefin", "gloria", "pacifico"];
var color = ["blue", "purple", "yellow", "red", "orange", "green"];
var background = ["bg-blue", "bg-purple", "bg-yellow", "bg-red", "bg-orange", "bg-green"];
$("#try-me").click(function() {
 $("#content")
 .hide()
 .removeClass()
 .addClass(font[Math.floor(Math.random()*font.length)]) // Font
 .addClass(color[Math.floor(Math.random()*color.length)]) // Color
 .fadeIn(600);
 $("html")
 .removeClass()
 .addClass(background[Math.floor(Math.random()*background.length)]); // Background
});

• Carlos: It's pretty clear and elegant as it is.
• jsanc623: I agree with Carlos, looks clear and elegant as it is.
• 200_success: Perhaps you should ensure that the randomly chosen background and foreground colours are different from each other.

Answer 1

The repetition of getting a random font, color, background class cries to be extracted to a utility function:

function pickRandom(arr) { 
 return arr[Math.floor(Math.random() * arr.length)];
}
$("#try-me").click(function() {
 $("#content")
 .hide()
 .removeClass()
 .addClass(pickRandom(font))
 .addClass(pickRandom(color))
 .fadeIn(600);
 $("html")
 .removeClass()
 .addClass(pickRandom(background));
});

I also agree with @MainMa about defining background in terms of color.

Other than that, I think it's nice code!

Ensuring change

If you want to make sure that each font/color/background are different every time the user clicks on the button, then perhaps you can create another helper:

function pickRandomExcept(arr, previous) { 
 while (true) {
 var pick = arr[Math.floor(Math.random() * arr.length)];
 if (pick != previous) {
 return pick;
 }
 }
}

However, in the caller you will need to keep track of the previous picks of each. For example:

var picked_color;
$("#try-me").click(function() {
 picked_color = pickRandomExcept(color, picked_color);
 $("#content")
 .hide()
 .removeClass()
 .addClass(pickRandom(font))
 .addClass(picked_color)
 .fadeIn(600);

Answer 2

One possible improvement consists of removing code duplication between those two arrays:

var color = ["blue", "purple", "yellow", "red", "orange", "green"];
var background = ["bg-blue", "bg-purple", "bg-yellow", "bg-red", "bg-orange", "bg-green"];

Instead, you can build the second array from the first one:

var color = ["blue", "purple", "yellow", "red", "orange", "green"];
var background = color.map(function (c) { return "bg-" + c; })

This has two benefits:

• If colors change, you have to do the change only once.

• If the source of colors change (for example you start loading colors from a database or a configuration file), you have to change only the first line.

On the other hand, the drawbacks are:

• That the code may be more difficult to understand by the beginners.

• That in your particular case, the duplication is not an important issue: there are only six colors and arrays are easily modifiable by hand.

• Flambino: Nice. You could also just prepend the "bg-" when adding the class, i.e. addClass('bg-' + ...), instead of mapping the array upfront. The benefit would be IE 6/7/8 support, since they don't have map

Answer 3

Well code look good, any changes will be just a over thinking. one thing I want to make here is to specify variable for that calculation.this will make a bit readable ( not a very big point though)

// Switch font randomly
var font = ["lobster", "shadows", "oswald", "josefin", "gloria", "pacifico"];
var color = ["blue", "purple", "yellow", "red", "orange", "green"];
var background = ["bg-blue", "bg-purple", "bg-yellow", "bg-red", "bg-orange", "bg-green"];
$("#try-me").click(function() {
var selectedFontClass=font[Math.floor(Math.random()*font.length)];
var selectedColorClass=color[Math.floor(Math.random()*color.length)];
var selectedBackgroundClass=background[Math.floor(Math.random()*background.length)];
$("#content")
 .hide()
 .removeClass()
 .addClass(selectedFontClass) // Font
 .addClass(selectedColorClass) // Color
 .fadeIn(600);
$("html")
 .removeClass()
 .addClass(selectedBackgroundClass); // Background
});

• jsanc623: On a personal note, I find the original version easier to read vs this version with variables (original looks cleaner).