Welcome to the Treehouse Community

Want to collaborate on code errors? Have bugs you need feedback on? Looking for an extra set of eyes on your latest project? Get support with fellow developers, designers, and programmers of all backgrounds and skill levels here with the Treehouse Community! While you're at it, check out some resources Treehouse students have shared here.

Looking to learn something new?

Treehouse offers a seven day free trial for new students. Get access to thousands of hours of content and join thousands of Treehouse students and alumni in the community today.

Start your free trial

JavaScript JavaScript Loops, Arrays and Objects Simplify Repetitive Tasks with Loops The Refactor Challenge

Sean Pascoe
Sean Pascoe
17,151 Points
Posted by Sean Pascoe
Sean Pascoe
17,151 Points

Refactor Runaway - probably got carried away with refactoring, but had fun. 

var numDivs = 10;
var html = '';
var rgbColor;


//generate pseudo-random number, 0-255 
function rand255() {
    return Math.floor(Math.random() * 256 );
}

//generate random color in rgb() format
function randColor() {
    var rgbCode = '';
    for ( j = 0; j < 3; j++ ) {
        if ( j < 2 ) {
        rgbCode += rand255() + ',';
        } else {
        rgbCode += rand255();
        }
    }
    return 'rgb(' + rgbCode + ')';
}

//create div with random background color
function newColorDiv() {
    return '<div style="background-color:' + randColor() + '"></div>';
}

//create html for defined number of random color divs (numDivs) 
function prettyLights() {
    for ( i = 0; i < numDivs; i++ ) {
        html += newColorDiv();
    }
    return html;
}

document.write( prettyLights() );
Sean Pascoe
Sean Pascoe
17,151 Points
Sean Pascoe
Sean Pascoe
17,151 Points

This isn't the most concise code, but all the parts are separated and I wrung it DRY (it would be easier to just define the three color codes manually, but eh, another loop was fun) Interesting thing I had to troubleshoot... it wasn't obvious at first, but I had the problem with a for loop running within another for loop. I had used the same variable ('i') for iteration in both loops and that obviously caused problems. That's why you'll see "j" used in the first one.

I like having convenient control over number of divs too.

Damien Watson
Damien Watson
27,419 Points
Damien Watson
Damien Watson
27,419 Points

The reason you have problems with the 'i' is because it is set to '0' in the first loop, then enters the 'prettyLights' where it goes up to '10', so when it comes back it is now equal to '10'. You can get around this by adding a 'var' in front of the definition. This makes it a localised variable:

for (var i = 0; i < 3; i++ ) {
    if (i < 2 ) {
        rgbCode += rand255() + ',';
    } else {
        rgbCode += rand255();
    }
}

There is a trick you can do to reduce this down more though its a little more advanced. I will drop it in and then explain:

for (var i = 0; i < 3; i++ ) {
  rgbCode += rand255()+(i<2?',':'');
}

// The extra part takes the 'if' statement and shorthands it,
// so this part is normal:
rgbCode += rand255()

// Then we add to it a comma if 'i' is less than '2'. The left hand part of
// the ? is the evaluation (equals true or false) and the right hand side is
// the value to assign (true:false).

// So it looks like:
var abc = "" + (evaluation ? true : false);

// You might also see:
var abc = (evaluation) ? true : false;

It might take a little to get used to but is quite cool to use.

Damien Watson
Damien Watson
27,419 Points
Damien Watson
Damien Watson
27,419 Points

Nice work btw, it's satisfying to see your code shrink down when refactoring. :)

Sean Pascoe
Sean Pascoe
17,151 Points
Sean Pascoe
Sean Pascoe
17,151 Points

Awesome! Thanks Damien Watson! Yea, I have no idea why I forgot the 'var' in the for loops... whoops. That shorthand 'if' statement is really cool.

Matt F.
Matt F.
9,518 Points
Matt F.
Matt F.
9,518 Points

I think it was a fine job of refactoring, as you did not sacrifice readability for brevity.

In case it is of use to you, I just wanted to let you know that you can rewrite

var numDivs = 10;
var html = '';
var rgbColor;

as

var numDivs = 10,
    html = '',
    rgbColor;