4

I am trying to make this script more concise, since I will be adding on more statements in the future.

x = Math.floor((Math.random() * 9) + 1);
var one = document.getElementById("test");

if(x === 1) {
  one.style.backgroundColor = "red";
}
if(x === 2) {
  one.style.backgroundColor = "blue";
}
if(x === 3) {
  one.style.backgroundColor = "yellow";
}
Bryson Noble
  • 337
  • 1
  • 11
  • 4
    If your code works and you seek constructive criticism or improvements, you should ask on [codereview.se]. –  Feb 12 '19 at 21:14
  • @Amy Agree. However, the fact is that "rule" is selectively or entirely ignored at SO [Return the last item in an array to first spot](https://stackoverflow.com/q/54624869/); [Recursively get all children](https://stackoverflow.com/questions/54644419/recursively-get-all-children) (thus could be ignored entirely or selectively in kind based on individual user judgement without running afoul or acting inconsistently with actual practice of the "rule"). – guest271314 Feb 12 '19 at 21:24
  • 1
    @guest271314 What is your point? There are always going to be people who ignore rules. That is not a reason to also disregard them. –  Feb 12 '19 at 21:58
  • @Amy Agree with you. You are correct. – guest271314 Feb 12 '19 at 22:00

5 Answers5

6

You can store the properties and values in a plain object

const o = {1:'a',2:'b',3:'c'}
one.style.backgroundColor = o[x]
guest271314
  • 1
  • 15
  • 104
  • 177
1

If you do not need the random value for other purpose, you could take an array and check if you got a truthy value for setting the color.

var x = ['red', 'blue', 'yellow'][Math.floor(Math.random() * 9)],
    one = document.getElementById("test");

if (x) one.style.backgroundColor = x;
<div id="test">test</div>
Nina Scholz
  • 376,160
  • 25
  • 347
  • 392
1

Another solution could be creating a Map between ids and colors. Note this enable the possibility of define values for non-consecutive indexes. Also, you could, for example, assign multiple mappings to some color to give they more probability.

let bgColor = new Map();
bgColor.set(1, {bg:"red", c:"black"});
bgColor.set(2, {bg:"blue", c:"white"});
bgColor.set(3, {bg:"green", c:"red"});
bgColor.set(4, {bg:"skyblue", c:"black"});
bgColor.set(5, {bg:"maroon", c:"white"});
bgColor.set(6, {bg:"red",c:"black"});
bgColor.set(7, {bg:"red",c:"black"});
bgColor.set(8, {bg:"BlueViolet", c:"white"});

var one = document.getElementById("test");

setInterval(function()
{
    let x = Math.floor((Math.random() * 9) + 1);
    let {bg, c} = bgColor.get(x) || {};        
    console.log(x, bg, c);

    if (bg)
    {
        one.style.backgroundColor = bg;
        one.style.color = c;
    }
}, 1000);
<div id="test">I'm Gonna be Iron Like a Lion in Zion</div>
Shidersz
  • 16,846
  • 2
  • 23
  • 48
0

Another possibility, if your end goal is to simply randomly select between a number of equally-weighted values is to write a simple function that accepts those values, and either returns one or returns a function, which when called returns one. Here's the second version:

const randomChoice = (vals) => () => vals[Math.floor(vals.length * Math.random())]
const randomColor = randomChoice(['red', 'blue', 'yellow'])

document.body.style.backgroundColor = randomColor()

Here randomChoice is a utility function that might be stored elsewhere, and randomColor generates one of the items you need.

This might not meet your needs. But if the three branches were just placeholders, and you would eventually need the nine the code suggests, then this might be the easiest to keep up-to-date.

This could be expanded to handle unequal weightings, but it would be more complex. There are plenty of places where this sort of random choice could be useful.

Olian04
  • 6,480
  • 2
  • 27
  • 54
Scott Sauyet
  • 49,207
  • 4
  • 49
  • 103
0

Another way of doing it would be by implementing a simple weight system.

const colors = [
  { color: 'skyblue', weight: 1 },
  { color: 'orangered', weight: 2 },
  { color: 'black', weight: 1 },
  { color: 'teal', weight: 3 }
].reduce((res, {color, weight}) => [...res, ...Array(weight).fill(color)],[]);

const randomColor = () => colors[Math.floor(colors.length * Math.random())];


// Demo
Array(50).fill().forEach(() => {
  const el = document.createElement('div');
  const color = randomColor();
  el.innerText = color;
  el.style.backgroundColor = color;
  document.body.append(el);
});
Olian04
  • 6,480
  • 2
  • 27
  • 54
  • This is nice. I was thinking weighting was going to be more difficult than this. You could also start from `{skyblue: 1, organgered: 2, black: 1, teal: 3}`, which might be easier to maintain. – Scott Sauyet Feb 12 '19 at 22:00
  • @ScottSauyet true. However this way allows for multiple entries of the same color, which might be desirable in some cases. But yes, its just a minor difference. – Olian04 Feb 12 '19 at 22:06