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

Marc Murray
Marc Murray
4,618 Points
Posted by Marc Murray
Marc Murray
4,618 Points

Refactoring challenge: My divs get printed 55 times, with the 10 colours. Can anyone spot where I'm going wrong?

Code here:

var html = '';
var red;
var green;
var blue;
var rgbColor;


function getValue() {
    var randomValue = Math.floor(Math.random() * 256);
    return randomValue;
}


function assignColor() {
    red = getValue();
    green = getValue();
    blue = getValue();
}

function printColor() {
    getValue();
    assignColor();
    rgbColor = 'rgb(' + red + ',' + green + ',' + blue + ')';
    html += '<div style="background-color:' + rgbColor + '"></div>';
    document.write(html);
}

for (var i = 1; i <=10; i += 1) {
    printColor();
}

4 Answers

Grace Kelly
Grace Kelly
33,990 Points
Grace Kelly
Grace Kelly
33,990 Points

Hi Marc, try moving the document.write() function outside of the printColor() function, like so:

function printColor() {
    getValue();
    assignColor();
    rgbColor = 'rgb(' + red + ',' + green + ',' + blue + ')';
    html += '<div style="background-color:' + rgbColor + '"></div>';
}

for (var i = 1; i <= 10; i += 1) {
    printColor();
}

document.write(html); //move outside of printColor() function

When you had it inside the printColor() function, it was continuously printing out the html during each iteration through the loop, hence you were getting 55 divs instead of 10, hope that helps!!

Grace Kelly
Grace Kelly
33,990 Points
Grace Kelly
Grace Kelly
33,990 Points

Alternatively if you wanted to keep the document.write() function where it is you can replace the += when assigning the <div> to the html variable with = , like so:

function printColor() {
    getValue();
    assignColor();
    rgbColor = 'rgb(' + red + ',' + green + ',' + blue + ')';
    html = '<div style="background-color:' + rgbColor + '"></div>'; //replace "+=" with "="
    document.write(html);
}

for (var i = 1; i <= 10; i += 1) {
    printColor();
}
Marc Murray
Marc Murray
4,618 Points
Marc Murray
Marc Murray
4,618 Points

That worked, thanks! :) Kind of confused about how the script runs now though, if the document.write() is outside of the loop, why does it then write 10 times? I thought the loop would be what made it happen that amount of times, now it seems to print the html variable the amount of times it gets changed?

Grace Kelly
Grace Kelly
33,990 Points
Grace Kelly
Grace Kelly
33,990 Points

It doesn't get written 10 times, rather the html variable is added to 10 times within the loop, using the printColor() function, to break this process down I'll use the following:

for (var i = 1; i <= 10; i += 1) {
    html += '<div style="background-color:' + rgbColor + '"></div>'; 
//div is added to html variable 10 times through loop within the printColor() function
}

document.write(html); //html variable containing 10 divs is printed out once

hope that clears things up a bit :)

Danny Fezer
Danny Fezer
4,116 Points
Danny Fezer
Danny Fezer
4,116 Points 
var html = '';

function randomColor() {
    var color = Math.floor(Math.random() * 256);
    return color;
}

var rgbColor = randomColor();

for (rgbColor = 1; rgbColor <= 10; rgbColor++) {
   html += '<div style="background-color:' + rgbColor + '"></div>';
}

document.write(html);
Ognjen Jevremovic
Ognjen Jevremovic
13,028 Points
Ognjen Jevremovic
Ognjen Jevremovic
13,028 Points

Hello Marc. How are you? I hope you're enjoying the journey through javascript courses. I know I did! I plan on going through each one of them soon, since I only took the ones that were in the "Front End Development Track".

Anyways, to get back on the track, lol. You're getting more than divs printed out on the page (than the number of times loop runs), because each time through the loop you're updating the value stored in the html variable; you're concatenating the string value (by taking the current value and adding a new value after it) so each time through the loop (except the first time) you're adding more than just one div to the web page. The operator should be " = " instead of " += ", which once again takes the current value of the variable and adds up the new value at the end of it.

I hope you don't mind I tweaked a code a lil' bit:

/* Outlined variables */
var html = '';
var red;
var green;
var blue;
var rgbColor;


/* Generate a random number between 0 and 255. Used later on as a value of an RGB color code */
function getValue() {
    return Math.floor(Math.random() * 256);
}

/* Update the values of red, green and blue variables, with a random number generated from getValue() function */
function assignColor() {
    red = getValue();
    green = getValue();
    blue = getValue();
}

/* Print the div html element to the web page each time through the loop and update the color styles of the div */
for (var i = 0; i < 10; i++) {
//    getValue();       < === the function doesn't need to be called in here
    assignColor();
    rgbColor = 'rgb( ' + red + ', ' + green + ', ' + blue + ' );';
    html = '<div style="background-color: ' + rgbColor + '></div>';
    document.write(html);
}

Hope that solves it out. Much success on your learning path Marc! Best wishes

Marc Murray
Marc Murray
4,618 Points
Marc Murray
Marc Murray
4,618 Points

Awesome, really in depth response, thanks!

Paul Brennan
Paul Brennan
8,809 Points
Paul Brennan
Paul Brennan
8,809 Points

If it's any comfort, the same thing was happening to me. Pesky +=. Bah.

Marc Murray
Marc Murray
4,618 Points
Marc Murray
Marc Murray
4,618 Points

Ha its a tricky one allright! Although mistakes like that are what teach you stuff about the language that might be glossed over in courses etc, getting it eventually always feels great!