Reputation: 219
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
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
Reputation: 4912
Because valueArr
is defined globally, it is not reset to []
each time randomValue()
is called and the next three values are just push
ed 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
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
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
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