2

I am trying to create a random color generator is there any way to shorten this code and convert into es6 arrow function? Thank you

let html = "";
let rgbColor;

function randomColors(red, green, blue) {
  for (let i = 1; 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>`;
  }

 document.write(html);
}

randomColors()
Felipe Skinner
  • 12,076
  • 2
  • 24
  • 29
Harman Pannu
  • 25
  • 11
  • Nope, arrow functions are always anonymous. – Teemu Dec 31 '17 at 21:31
  • 2
    [See the question](https://stackoverflow.com/questions/22939130/when-should-i-use-arrow-functions-in-ecmascript-6). It's generally recommended to use just use `function` when you are in a global scope or for methods. There isn't much of a benefit to use it in this case. One thing you could do is separate out `Math.floor(Math.random() * 256)` into a function and call that. Other than that there isn't much to shorten here. – Spencer Wieczorek Dec 31 '17 at 21:36
  • 1
    No, an arrow function does not shorten this code. No reason to use it when you don't need the concise body or lexical `this`. – Bergi Dec 31 '17 at 22:39
  • What is best scenario to use arrow function as I am still new to JS It’s really hard to wrap my head around it. – Harman Pannu Dec 31 '17 at 22:43
  • To clarify my last comment as I missed a word, I meant it generally *not* recommended. – Spencer Wieczorek Dec 31 '17 at 23:20

5 Answers5

4

You can use Array#from and Array#join to generate each color, and the list of colors, and combine everything to a string:

const randomColors = (count) => 
  Array.from({ length: count }, (_, k) => // create an array of divs with length of count
    `<div style="background-color: rgb(${
      Array.from({ length: 3 }, () => Math.floor(Math.random() * 256)).join()
    })">${k}</div>` // inside each div create an array of 3 colors, and join them
  ).join(''); // join the array of divs to one string

document.write(randomColors(9));
Ori Drori
  • 145,770
  • 24
  • 170
  • 162
  • I like this solution but too complicated for a beginner like me :) – Harman Pannu Dec 31 '17 at 21:54
  • @HarmanPannu - it's pretty simple actually. Instead of producing each color separately, I create an array of 3 colors (using Array#from) and combine them to a string using join. I've placed the expression inside the template string of the div. The outer Array does the same thing, but with the divs array. You should read the links in the answer. – Ori Drori Dec 31 '17 at 21:58
  • 1
    I get it now looked really complicated at first sight but I tried to re-write it on my own now it makes sense to me :) – Harman Pannu Dec 31 '17 at 22:07
1

You can do it like this:

let html = "";
let rgbColor;

let randomColors = (red, green, blue) => {
  for (let i = 1; 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>`;
  }
  document.write(html);
}

randomColors()

But this will not shorten this code too much.

Sergii Rudenko
  • 2,210
  • 1
  • 16
  • 21
  • How can I put random number generator in one function rather than repeating the code 3 times? – Harman Pannu Dec 31 '17 at 21:48
  • @HarmanPannu I have posted an alternate solution that does so. It requires abstracting out the logic for `Math.floor(Math.random() * 256)` into a separate function. – IsenrichO Dec 31 '17 at 21:50
1

This answer is accurate inasmuch as it is a valid representation of your randomColors function that uses arrow functions. However (and as he notes), it alone does not provide much in the way of shortening the code. For that, I would instead propose pulling out a helper function for generating your random color (see the getRandColor function below).

let html = "";
let rgbColor;

const randomColors = (red, green, blue) => {
  const getRandColor = () => Math.floor(Math.random() * 256);

  for (let i = 1; i <= 10; i++) {
    [red, green, blue] = [getRandColor(), getRandColor(), getRandColor()];
    rgbColor = `rgb(${red},${green},${blue})`;
    html += `<div style="background-color:${rgbColor}"></div>`;
  }
  document.write(html);
}

Try the above instead, perhaps.

IsenrichO
  • 3,193
  • 3
  • 14
  • 30
1

Shorter isn't always best. This is pretty short, but it can be a bit opaque:

let html = ""

const randomColors=()=>(
    ((rnd)=>(`<div style="background-color: rgb(${rnd()},${rnd()},${rnd()})"></div>`))
    (()=>Math.floor(Math.random()*256))
)

html+=randomColors()
document.write(html)
Ryan Hanekamp
  • 378
  • 2
  • 5
0

ES6 arrow functions are used to replace standard anonymous functions without creation new scope that functions usually create. So your function will remain its name and won’t be an arrow function.

Sergey
  • 5,085
  • 4
  • 29
  • 61