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 Solution

Martin Wiulsrød
seal-mask
.a{fill-rule:evenodd;}techdegree seal-36
Martin Wiulsrød
Front End Web Development Techdegree Graduate 16,314 Points
Posted by Martin Wiulsrød
.a{fill-rule:evenodd;}techdegree seal-36
Martin Wiulsrød
Front End Web Development Techdegree Graduate 16,314 Points

What other improvements can I do?

Hello! I spent quite a lot of time on refactoring this code, it was really good practice! Below is the code that I ended up with. I'm certain there is an even better way of doing this and I'm open to any suggestions. Thanks!

// define variable
var html = '';

// generate a random number between 0 and 256
function randomColor() {
  return Math.floor(Math.random() * 256 + 1)
}

// create 10 div's containing 3 different rgb values each time, the value is created using the randomColor() function
for (var i = 0; i < 10; i++) {
  html += '<div style="background-color:' + 'rgb(' + randomColor() + ',' + randomColor() + ',' + randomColor() + ')' + '"></div>';
}

// write the div to the html site
document.write(html);

3 Answers

Liam Clarke
Liam Clarke
19,938 Points

Looks good Martin.

What you could think about now is reusability.

Think about how you can make this reusable throughout your example, maybe with another function? I might want to display 10 random divs on the page load but then reshuffle then on a click of a button too.

I can do this by calling the function on load and the function on click on a button

Martin Wiulsrød
seal-mask
.a{fill-rule:evenodd;}techdegree seal-36
Martin Wiulsrød
Front End Web Development Techdegree Graduate 16,314 Points
Martin Wiulsrød
.a{fill-rule:evenodd;}techdegree seal-36
Martin Wiulsrød
Front End Web Development Techdegree Graduate 16,314 Points

That is a really good tip and something that I feel is easy to forget as a beginner. I tried to split up the different functionalities. What do you think?

// generate a random number between 0 and 256
function randomColor() {
  return Math.floor(Math.random() * 256 + 1)
}

// print out a message
function print(message) {
  document.write(message);
}

// create a div and change background color to a random rgb value using the randomColor() function
function displayDivs() {
  for (var i = 0; i < 10; i++) {
  print('<div style="background-color:' + 'rgb(' + randomColor() + ',' + randomColor() + ',' + randomColor() + ')' + '"></div>');
  }
}

// call function
displayDivs();
Liam Clarke
Liam Clarke
19,938 Points
Liam Clarke
Liam Clarke
19,938 Points

Nice! you've got it and with an eventListener, we can now use the displayDivs() function to reuse the function and reshuffle as many times as we want.

Something I changed was document.write, we don't want to rewrite the whole document when we click the button so lets contain the colours.

// generate a random number between 0 and 256
function randomColor() {
    return Math.floor(Math.random() * 256 + 1)
}

// print out a message
function print(message) {
    document.getElementById("colour").innerHTML = message;
}

// create a div and change background color to a random rgb value using the randomColor() function
function displayDivs() {
    var html = "";
    for (var i = 0; i < 10; i++) {
        html += '<div style="background-color:' + 'rgb(' + randomColor() + ',' + randomColor() + ',' + randomColor() + ')' + '"></div>';
    }
    print(html);
}

// call function
displayDivs();

document.getElementById("colourShuffle").addEventListener("click", function () {
    displayDivs();
});

    <button id="colourShuffle">
        Shuffle Colours!
    </button>
    <div id="colour"></div>

There are other areas that could be improved but they seem out of the scope of where you are at, that being said your miles ahead of this module so keep up the good work and maybe come back to this later on!

Good Luck!

Siddharth Pande
Siddharth Pande
9,046 Points
Siddharth Pande
Siddharth Pande
9,046 Points 
var html = '';
var red;
var green;
var blue;
var rgbColor;

function colorCircles() {
 for ( var k = 0; k < 10; k +=1) {
   var j = 0
   var rgbColor = "rgb("; 
   for (i=1; i<=3; i+=1){ 
    var colorValue = Math.floor(Math.random() * 256 );
    rgbColor += colorValue;
    while (j < 2) { 
      rgbColor += ",";
        break;
      }
     j += 1;
    }
    rgbColor += ")"; 
    console.log(rgbColor);
    html += '<div style="background-color:' + rgbColor + '"></div>';
  }

  return html;
}


document.write(colorCircles());