Salam.MSaif
Salam.MSaif

Reputation: 219

All generated random value is same

This is the code that I wrote, I dont know why all the 50 rgbColor generate the same value even when I wrote the random function in randomRGBValue.

var html = '';
var valueArr = [];
var rgbColor;

function randomValue() {
  for (var i = 0; i < 3; i += 1) {
    valueArr.push(Math.floor(Math.random() * 256 ));
  }
  return valueArr;
}

function randomRGB() {
  randomValue();
  var color;
  color = 'rgb(';
  color += valueArr[0] + ',';
  color += valueArr[1] + ',';
  color += valueArr[2] + ')';
  return color;
}

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

document.write(html);

To ease understanding of my poor written code,

-> randomValue function is for generating random value from 1 to 255(3 sets) and is pushed into valueArr. valueArr is returned.

Example valueArr = (value1, value2, value3)

-> randomRGB function calls randomValue function and plugs in valueArr value into the color array.

Upvotes: 2

Views: 102

Answers (5)

Slai
Slai

Reputation: 22866

Hex color can be a bit easier:

for (var i = 0; i < 10; i += 1) 
    document.write('<div style="background-color:#' + 
        ('00000'+(Math.random()*(1<<24)|0).toString(16)).slice(-6) + '">.</div>');

Upvotes: 0

ContinuousLoad
ContinuousLoad

Reputation: 4912

Because valueArr is defined globally, it is not reset to [] each time randomValue() is called and the next three values are just pushed to the end of valueArr (not at all modifying the first three values).

As a result, when randomRGB() constructs its color string, it looks at the first three values in valueArr (which are always the same since the newly generated random values are being appended to the end of the array).

Two solutions:

(1) make valueArr a local variable defined inside randomValue()

function randomValue() {
  var valueArr = [];
  ...
}

(2) keep valueArr where it is, but reset it to [] in randomValue()

var valueArr;
...
function randomValue() {
  valueArr = [];
  ...
}

Hope this helps!

Upvotes: 3

raksa
raksa

Reputation: 938

hi you can shorten it like this

function randomRGB() {
  return 'rgb(' + [
    Math.random() * 256 | 0,
    Math.random() * 256 | 0,
    Math.random() * 256 | 0
  ].join(',') + ')';
}
var html = '';
for (var i = 0; i < 50; i++) {
  html += '<div style="background-color:' + randomRGB() + '"></div>';
}
console.log(html);

Upvotes: 3

karthikdivi
karthikdivi

Reputation: 3633

Move globally declared variables valueArr, rgbColor into the respective functions Check the working code here Demo: https://codepen.io/karthikdivi/pen/ZJJeoa?editors=0010

var html = '';
//var valueArr = [];
//var rgbColor;

function randomValue() {
  var valueArr = [];
  for (var i = 0; i < 3; i += 1) {
    valueArr.push(Math.floor(Math.random() * 256 ));
  }
  return valueArr;
}

function randomRGB() {
  var valueArr = randomValue();
  var color;
  color = 'rgb(';
  color += valueArr[0] + ',';
  color += valueArr[1] + ',';
  color += valueArr[2] + ')';
  return color;
}

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

document.write(html);

Upvotes: 2

Christopher Messer
Christopher Messer

Reputation: 2090

Your problem is that you never reset your valueArr - you keep making it bigger and bigger. But in your randomRGB() function, you always access the first three elements of the array.

You can fix this by just adding var valueArr = [] inside of your randomValue() function and you should be fine.

Upvotes: 6

Related Questions