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, Part 2

huckleberry
huckleberry
14,636 Points

My Solution... well, 2 solutions. I had not thought of a more split modular approach.

function rngColors(){
     var red = Math.floor(Math.random() * 256 );
     var green = Math.floor(Math.random() * 256 );
     var blue = Math.floor(Math.random() * 256 );
     return 'rgb(' + red + ',' + green + ',' + blue + ')';
}
var html ='';
var rgbColor;

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

document.write(html);

Of course that was still not DRY since I was repeating the <code>Math.random</code> method, but I did try it only a little different after that.

function rngColors(){
     var red = Math.floor(Math.random() * 256 );
     var green = Math.floor(Math.random() * 256 );
     var blue = Math.floor(Math.random() * 256 );
     return 'rgb(' + red + ',' + green + ',' + blue + ')';
}
var html ='';


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

document.write(html);

The difference being that this time I didn't declare a variable named <code>rgbColor</code> and assign a function call to <code>rngColors()</code> inside that for every iteration and put the variable inside the <code>html +=</code> expression.

That time I just dropped the variable all together and called the function directly from the <code>html +=</code> expression.

Then I realized I could do more so I went shorter.

function rgColors(){
     var rgb = 'rgb(' + Math.floor(Math.random() * 256 ) + ',' + Math.floor(Math.random() * 256 ) + ',' + Math.floor(Math.random() * 256 ) + ')';
     return rgb;
}
var html ='';
var rgbColor;

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

document.write(html);

This really cut down the code because I just cut down everything into a 2 line function, named a couple of variables and then a 2 line loop. This used the same concept I did in the first attempt up there at the top. It involved creating a variable and then assigning a function call as its value inside the loop.

My final revision was a mirror of my 2nd attempt up top where I eliminated the 2nd variable and just used the function call inside the <code>html +=</code>, as you can see below...

function rgColors(){
     var rgb = 'rgb(' + Math.floor(Math.random() * 256 ) + ',' + Math.floor(Math.random() * 256 ) + ',' + Math.floor(Math.random() * 256 ) + ')';
     return rgb;
}
var html ='';

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

    html += '<div style="background-color:' +  rgColors() + '"></div>';
}

document.write(html);

Anyway, any critique or pointers would be nice.

Cheers,

Huck - :sunglasses:

p.s. Fantastic lessons Dave McFarland , love your stuff.

Hi Huck,

Your process is fairly similar to mine, when thinking about refactoring. I liked the 2nd version - from the top - because of readability. Then I started thinking about how to refactor the generating of random numbers and this is what I came up with.

Thanks for sharing!

function rng(n) {
    return Math.floor(Math.random() * n);
}

function rngColors() {    
    return 'rgb(' + rng(256) + ',' + rng(256) + ',' + rng(256) + ')';
}

var html = '';

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

document.write(html);
Dave McFarland
Dave McFarland
Treehouse Teacher

Great job fellows!

huckleberry Both your solutions are great. I like the second one better.

Robert Richey Breaking out the random number generation into the generic rng function is a great idea. You may want to use that function to generate other random numbers for other purposes in the script. Also, simply returning the results of the expression -- return 'rgb(' + rng(256) + ',' + rng(256) + ',' + rng(256) + ')'; -- simplifies it even more.

Viva JavaScript!

Greg Kaleka
Greg Kaleka
39,021 Points

Hey, if we're sharing solutions - how about this crazy thing: I defined a function inside another function. I wasn't sure if it would work, but it does! This actually makes the random color function much more portable. The whole thing is self contained, and spits out random RGB without depending on any outside code:

function randomRGBColor() {
  function surprise() {
    return Math.floor(Math.random() * 256 );
  }
  return 'rgb(' + surprise() + ','  + surprise() + ',' + surprise() + ')'; 
}

var html= '';

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

document.write(html);

4 Answers

Ognjen Jevremovic
Ognjen Jevremovic
13,028 Points

Hello Adiv, how are you? Sweet solution. I like your approach. I do like looking at others people code, as it really impresses me how one problem could possibly have hundreads of different approaches, some of which I wouldn't even come up in years. I really do like that part of programming = ).

Well, as far as I can see, the erorr that caught my eye was your for in loop.

for (color in colors) {
      colors.color = getRGBValue();
   }

The thing is, that as you probably know already, for in loop is a special type of loop that can be used to itterate over object's properties, itterating through each and every propertie of an object(no skipping like you can do in arrays, like calling every 5th or so item). Now the thing is, that you can only use a square bracket notation inside of a for in loop. Let me demostrate a few examples in a code block below.

for ( var propName in yourObject ) {
  alert(yourObject[propName]);
}

Or in this particular example it would look like this:

for ( var color in colors) {
      colors[color] = getRGBValue();
   }

Now, it's just the syntax of a for in loop. But just to give you a few "Why is it, as it is" clues, as it would be much easier for you to understand the syntax, rather than by learning it all by heart.

//Let's make a cat object (*MEOW*)
var cat = {
name: 'SuperNyan';
color: '100% Nyan',
says: 'meow'
};


//Now let's just call for a value associated with a name propertie of a cat object that would like like this
cat.name;

//And that will return 'SuperNyan' string value

But what if we wanted to actualy retrieve all of the values from that cat object (cat's name, color and the actual sound she makes). We would need to use a for in loop for that and it would look something like this

for ( var prop in cat ) {
  console.log( cat[prop] );
  //It will write in the console: 'SuperNyan', '100% Nyan' and 'meow'
}

Now why did we use a [] instead of . notation. Let me explain a bit further. If we did something like this:

for ( var prop in cat ) {
  console.log( cat.prop );
} 

That would actualy return: undefined, undefined and undefined; but why so? As you see, when you use the dot notation, like we did in this example, that tell's the web browser (or the actual JS interperteur inside of a web browser) to look for a value inside of a cat object that is associated with a key named prop. Further more, that is actualy telling the web browser to look for a property with a name of prop and return a value associated with it which is, as you can guess, undefined. Why so you may wonder? It's because we didn't actualy define the prop key for our cat object, meaning that there's no value associated with that property. The same result would occur if we used

for ( var prop in cat ) {
  console.log( cat['prop'] );
}

Notice those ' ' (quotes) forming a string value? It is again telling the web browser to look for a property with a name of prop, inside of a cat object and return the value associated with that key, meaning something like this:

cat = {
name: 'SuperNyan';
color: '100% Nyan',
says: 'meow',
prop:     //  <------  It is looking for that, which does not exist.
};

I hope that you was able to follow me, as if not, feel free to buzz me up again and I'll come up with a better explanation and a more appropriate example.

Best wishes and

Keep on hacking!

Wow! That was a fascinating and very cogent explanation of the issue! Thank you so much for the detailed discussion. I think I get it now.

When using the dot notation, the property that follows it must be a literal and not a variable. But when using the [] notation we can place a variable inside the brackets and the JS interpreter will use that variable's current value to look up the property in the object.

It now makes PERFECT sense and explains why the result of getColors() was always 'rgb(0, 0, 0)'. The colors object NEVER gets updated if dot notation is used because the JS interpreter will look for a property named color in the colors object. But there is no such property! However the value corresponding to the blue property in the colors object can be found either by the expression colors[blue] or colors[prop] when prop === blue, such as when in the enhanced for loop. COOL!!

Thanks a lot! This really helped a great deal!

Cheers!

Ognjen Jevremovic
Ognjen Jevremovic
13,028 Points

I actually put myself into this task and tried to solve it as many times as I could think of, before looking at the Dave's solutions. So, here are some of my workarrounds (all DRY coded, but some are more DRY than the others), before even looking at the videos in which Dave shared his own solution.

With function invoking

///DRY Code by using FUNCTIONS!

function generateColorCircle() {
  red = Math.floor(Math.random() * 256 );
  green = Math.floor(Math.random() * 256 );
  blue = Math.floor(Math.random() * 256 );
  rgbColor = 'rgb(' + red + ',' + green + ',' + blue + ')';
  html += '<div style="background-color:' + rgbColor + '"></div>';
}

generateColorCircle();
generateColorCircle();
generateColorCircle();
generateColorCircle();
generateColorCircle();
generateColorCircle();
generateColorCircle();
generateColorCircle();
generateColorCircle();
generateColorCircle();

With while loop

///DRY code by using WHILE LOOP

var counter = 0;
while ( counter < 10 ) {
  red = Math.floor( Math.random() * 256 );
  green = Math.floor( Math.random() * 256 );
  blue = Math.floor( Math.random() * 256 );
  rgbColor = 'rgb(' + red + ',' + green + ',' + blue + ')';
  html += '<div style="background-color:' + rgbColor + '"></div>';
  counter++;
}

With do while loop

///DRY code by using DO WHILE LOOP

var counter = 0;
do {
  red = Math.floor( Math.random() * 256 );
  green = Math.floor( Math.random() * 256 );
  blue = Math.floor( Math.random() * 256 );
  rgbColor = 'rgb(' + red + ',' + green + ',' + blue + ')';
  html += '<div style="background-color:' + rgbColor + '"></div>';
  counter++;
} while ( counter < 10 )

With for loop

///DRY code by using FOR LOOP

for ( var i = 0; i < 10; i++ ) {
  red = Math.floor( Math.random() * 256 );
  green = Math.floor( Math.random() * 256 );
  blue = Math.floor( Math.random() * 256 );
  rgbColor = 'rgb(' + red + ',' + green + ',' + blue + ')';
  html +='<div style="background-color:' + rgbColor + '"></div>';
}

Some workarround (making the code even more DRY)

function getRandomNumber() {
  return Math.floor( Math.random() * 256 );
}


for ( var i = 0; i < 10; i++ ) {
  red = getRandomNumber();
  green = getRandomNumber();
  blue = getRandomNumber();
  rgbColor = 'rgb(' + red + ',' + green + ',' + blue + ')';
  html += '<div style="background-color:' + rgbColor + '"></div>';
}

Final solution I made (even more DRY)

function getRandomNumber() {
  return Math.floor( Math.random() * 256 );
}

for ( var i = 0; i < 10; i++ ) {
  rgbColor = 'rgb(' + getRandomNumber() + ',' + getRandomNumber() + ',' + getRandomNumber() + ')';
  html += '<div style="background-color:' + rgbColor + '"></div>';
}

Cheers!

P.S. Hit me up with your unique solution, as I would be more than glad to take a look at even more "funkier" way of solving this puzzle.

MONICA BUI
MONICA BUI
3,526 Points

AWESOME JOB Ognjen! I actually came up with a solution similar to your "workarround" version

function getColor() {
        return Math.floor(Math.random() * 256 );
    }

for (var i = 0; i <10; i++) {

    var red = getColor();
    var green = getColor();
    var blue = getColor();
    rgbColor = 'rgb(' + red + ',' + green + ',' + blue + ')';
    html += '<div style="background-color:' + rgbColor + '"></div>';
}

But I didn't come as far as your final solution which I think is terrific, it really gave me an "oh...nod nod!" moment!

function getColor() {
        return Math.floor(Math.random() * 256 );
    }

for (var i = 0; i <10; i++) {

    rgbColor = 'rgb(' + getColor() + ',' + getColor() + ',' + getColor() + ')';
    html += '<div style="background-color:' + rgbColor + '"></div>';
}

Also I noticed that Robert Richey's idea of giving the function a parameter then pass 256 as an argument when he called the function is fantastic because it make the code more reusable! Although we don't practically need it for this challenge it's still a good practice of always making sure our codes are both DRY & Reusable:

function getColor(n) {
        return  Math.floor(Math.random() * n );
    }

for (var i = 0; i <10; i++) {

    rgbColor = 'rgb(' + getColor(256) + ',' + getColor(256) + ',' + getColor(256) + ')';
    html += '<div style="background-color:' + rgbColor + '"></div>';
}

Regards!

Monica

Ognjen Jevremovic
Ognjen Jevremovic
13,028 Points

Hey Monica. I'm really sorry for my late response. Seems like I missed the notification about your comment.

Ahhh, really nice. Your code maybe looks 'longer' in our eyes, but trust me, because of that it is much more readable which really makes it a better solution! As don't forget we want to make our code readable and maintanable and actualy be able to understand it, even months from now.

I do like experementing and I do like to take my time to try to come up with at least 2-3 examples, before I watch Dave (or Andrew, later on in the jQuery basics course in the Front-End Development track) doing it himself. As it really gives me a wider spectar and it makes the code a bit more of 'my own'. But most of all, it's more understandable (at least to me) so I can probably write that same code after months from now, without the need to look inside of a documentation (or a refference), because it was me who hacked it arround.

Once again, it really impresses me how we all think of a different way of coming up with an exact same solution to a simple problem like this one. There's a bunch of unique ways that we made it work and we all did it somewhat different manner it above all, it does WORK!

Cheers Monica! I hope I'll see you later in the jQuery basics as there is a lot of potential of unlocking the true power of JS in there. spoiler alert

Michael Plemmons
Michael Plemmons
9,393 Points

Hey Monica,

my solution is exactly like one of yours. The simplest and less repetitive method I could think of.

var html = ''; var rgbColor;

function color() {

return Math.floor(Math.random() * 256 ); } for (var i=0; i<100; i++) {

rgbColor = 'rgb(' + color() + ',' + color() + ',' + color() + ')'; html += '<div style="background-color:' + rgbColor + '"></div>'; }

document.write(html);

I'd like to share my answer, but it took reading this thread and some creativity to come up with it. I wanted nothing to be reused, but everything to be usable. I have both generic and specific functions for not repeating any code, though I think I may have overdone it. While this is a simple example, I do believe this may be how more complicated code structures work.

function ranNum( min, max ) { 
  return Math.floor(Math.random() * (max - min) + min); 
}
function print( html ) { 
  document.write( html ); 
}
Function ranNum256() { 
  return ranNum(1, 256); 
}
function printDot(red,green,blue) { 
  print( '<div style="background-color:rgb(' + red + ',' + green ',' + blue + ')"></div>' ); 
}
function printRandomDot() { 
  printDot( ranNum256(), ranNum256(), ranNum256() ) 
}

var numOfDots = 10;

for( var i = 0; i < numOfDots; i++ ) {
  printRandomDot();
}

It's not strictly relevant to this course, but it's important to note that the RGB values are from 0 to 255, not 1 to 256.

Also, in your code you have a capital F for the function keyword for ranNum256. I think that would create a syntax error, or complain about an undefined variable.

Otherwise, this is an excellent use of functions and great practice for the future!

The various solutions presented here are fantastic and show really creative thinking. I tried to do the same but perhaps am getting ahead of myself. I created an object called colors to contain red, green and blue as the property values or keys and initialized the values to 0, which would result in rgb(0, 0, 0).

For some reason the rgb color string is ALWAYS coming out to rgb(0, 0, 0) despite the fact that random numbers are assigned to the colors object every time the getColors() function is called. Not sure where I am faltering here but I would appreciate any advice. Thank you

var html = '';

//object to hold RGB values
var colors = {red : 0,
              green : 0,
              blue : 0
              }

function getRGBValue() {
    //generate random values between 0-255 for the color values
    return Math.floor(Math.random() * 256);
}//getRGBValue()

function getColors() {
   //assign color values
  for (color in colors) {
      colors.color = getRGBValue();
   }
   return 'rgb(' + colors.red + ',' + colors.green + ',' + colors.blue + ')';
};//getCoolors()


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

document.write(html);
//getColors() always returns "rgb(0, 0, 0)" for some reason