Welcome to the Treehouse Community

The Treehouse Community is a meeting place for developers, designers, and programmers of all backgrounds and skill levels to get support. Collaborate here on code errors or bugs that you need feedback on, or asking for an extra set of eyes on your latest project. Join thousands of Treehouse students and alumni in the community today. (Note: Only Treehouse students can comment or ask questions, but non-students are welcome to browse our conversations.)

Looking to learn something new?

Treehouse offers a seven day free trial for new students. Get access to thousands of hours of content and a supportive community. Start your free trial today.

JavaScript

Nathaniel Nasarow
Nathaniel Nasarow
10,291 Points

Javascript code refactoring challenge. How is it? Thoughts?

First, github: site: http://torgian.github.io/for-loops-colored-divs code: https://github.com/Torgian/for-loops-colored-divs

So I started today with this challenge and I feel pretty good about myself. Was able to refactor the repeating code into a single for loop and function without having to review anything that I learned yesterday. A good feeling.

Thoughts on the code? Critique? Any better way to do it with the way I learned so far?

I thought about using a do / while loop first, but I think the for loop is a bit more efficient.

I also thought about putting the div code into the for loop. However, there are a couple reasons I decided not to do that.

One: it seems more user/programmer friendly to make the function separate from the for loop. In the for loop, all I'm doing is calling the function.

Two: If this was a larger program, it gives me the opportunity to use the var colorDiv = function () in other parts of the program, and not just in one for loop.

Thoughts on this opinion of mine?

1 Answer

First recommendation is to use js-code style guide. You can make it readable with js-beautifier. The second one - your function should return something or change something with parameters. It's better practice.

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

var colorDiv = function() {
    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>';
}

for (var i = 0; i <= 20; i += 1) {
    colorDiv();
}

When your code will grow it will be hard to understand what colorDiv function do and what it's affect on. For better style of your code you will convert "Math.floor(Math.random() * 256);" in to the separate function but it will be in the next chapter (as I remember).

Look at the previous question in the JS-forum. At the end you will post the result html to the page. Don't use document.write inside colorDiv function. Investigation "why it's bed way" you'll find in my answer for the previous question.

Nathaniel Nasarow
Nathaniel Nasarow
10,291 Points

Right? :D

So I just finished the challenge. I had to look at the teacher's solution before I could completely understand what I should have done. I had the correct idea.. making a function out of the Math.random line of code, but I could not exactly figure out how to do it correctly.

Yum. Learning.

Either way, I just put it into practice and updated my github code. Starting to make even more sense now.

Yup. "Right" is depend on what you are trying to make. It's solve the quizz? You are right and updated code is more readable. Good job.)