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 Working with 'for' Loops The Refactor Challenge – Duplicate Code

Samantha Atkinson
seal-mask
.a{fill-rule:evenodd;}techdegree seal-36
Samantha Atkinson
Front End Web Development Techdegree Graduate 40,307 Points

My Solution - Not sure if I've done bad practice in the way I have done my arrow function?

This is my code and it works. I would like to know if I created or used the arrow functions in a bad practice kind of way.

let html = '';

const randomValue = () => Math.floor(Math.random() * 256);
const color = () =>`rgb( ${randomValue()}, ${randomValue()}, ${randomValue()} )`;

for (let i = 1; i <= 10; i++) {
    html += `<div style="background-color: ${color()}">${i}</div>`;

}

document.querySelector('main').innerHTML = html;

4 Answers

Steven Parker
Steven Parker
231,269 Points

Concise, efficient, and readable — good job! :+1:

Steven Parker
Steven Parker
231,269 Points

Since "map" converts an existing collection into a different one, and no collection is being used here, I wouldn't think it would be a good choice for this situation. But after Jonathan's suggestion and Tadjiev's question, I was thinking about how it might be possible to use it here:

const color = () =>`rgb(${[1,2,3].map(_=>randomValue()).join(",")})`;

But while it does work, this is a rather strange and obscure use of "map" and I would not recommend it as a solution.

Looks good! If you want to take it one step further and be able to pass in that first function as an argument like Guil mentions towards the end of the video, you can update that second anonymous function and still keep your arrow syntax like so:

const color = value => `rgb( ${value()}, ${value()}, ${value()} )`;

And then simply pass in the randomValue as an argument when you call it:

${color(randomValue)}
Steven Parker
Steven Parker
231,269 Points

I'm having a hard time imagining what the benefit of this indirection would be.
When would you ever inject any other function than "randomValue"?

Hi, Steven. :wave: It was simply to follow the end of the video (which was intended to demonstrate how you could pass functions to other functions as arguments), and posted for anyone who might be curious regarding how to keep OP's arrow syntax given Guil's finished example instead of using a function declaration.

You're correct in that there are few (to no) use cases where this would be a desired implementation for this specific example. The only other use might be if you had perhaps defined a non-randomly generated RGB color value that used the same integer for all red, green, and blue values (which - you're definitely correct - seems oddly specific and highly unlikely to ever be the case).

However, since the goal here was not to learn a specific example of generating an RGB value, but instead to build upon general JS learning at this point, I thought I'd include it. :thumbsup:

Johnathan Guzman
Johnathan Guzman
7,432 Points

Hi Samantha ! Personally, I prefer to wrap my code blocks in brackets. I believe that is the way to go, other than that, good job ! Also, instead of using for loops try using the map method !

Samantha Atkinson
seal-mask
.a{fill-rule:evenodd;}techdegree seal-36
Samantha Atkinson
Front End Web Development Techdegree Graduate 40,307 Points

Thank you, Johnathan, for taking the time to answer. I didn't use brackets because the code block can fit on one line, otherwise, I would have used brackets. I haven't learnt the map method yet, but looking forward to learning about it. ; )

Steven Parker
Steven Parker
231,269 Points

Also remember that the form that uses brackets also requires an explicit "return" statement. In the compact form used here the "return" is done for you.

Tadjiev Codes
Tadjiev Codes
9,626 Points

But how we could use the map method in this example? I've used it to iterate over arrays but here it's a bit different. Thanks

Steven Parker
Steven Parker
231,269 Points

I found this suggestion curious as well, but see the comment I added to my answer.

Brendan Moran
Brendan Moran
14,052 Points

I had a very similar solution! I nested my value function inside of my color function, otherwise we were thinking much the same way.

let html = '';

function getRandomRGB () {
  const randomValue = () => Math.floor(Math.random() * 256);
  return `rgb(${randomValue()}, ${randomValue()}, ${randomValue()})`;
}

for(let i = 1; i <= 10; i++) {
  html += `<div style="background-color: ${getRandomRGB()}">${i}</div>`;
}

document.querySelector('main').innerHTML = html;
Tadjiev Codes
Tadjiev Codes
9,626 Points

Thanks, Mr.Steven) I think for me as a beginner again For Loop would be easier to understand especially in this situation. Thank you to explain how it would work anyways with the map)