Optimizing code to map numbers 1 - 25 uniformly to 5 colors

Question

I am currently learning JavaScript, and I made this code (and it works fine) but I know that there must be a way to automate this better. I'm little bit fuzzy on loops, but from what I know of them, I don't know that one could be used in this situation:

function calculation() {

var num = document.getElementById("input").value;
var trCalc = num % 25;

var tipColorName = document.getElementById("tipColorName");
var ringColorName = document.getElementById("ringColorName");

var tipBlock = document.getElementById("tipBlock");
var ringBlock = document.getElementById("ringBlock");

// Pair Calculations

if (trCalc > 0 && trCalc <= 5) {
    tipColorName.innerHTML = "White";
    tipBlock.setAttribute("style", "background-color: #f2f2f2; color: #2f2f2f;");
}
if (trCalc > 5 && trCalc <= 10) {
    tipColorName.innerHTML = "Red";
    tipBlock.setAttribute("style", "background-color: #e24341; color: white;");
}
if (trCalc > 10 && trCalc <= 15) {
    tipColorName.innerHTML = "Black";
    tipBlock.setAttribute("style", "background-color: #2f2f2f; color: white;");
}
if (trCalc > 15 && trCalc <= 20) {
    tipColorName.innerHTML = "Yellow";
    tipBlock.setAttribute("style", "background-color: #f4ca37; color: #2f2f2f;");
}
if (trCalc > 20 && trCalc <= 24 || num % 25 == 0 && num > 0 ) {
    tipColorName.innerHTML = "Violet";
    tipBlock.setAttribute("style", "background-color: #844ae0; color: white;");
}
if ([1, 6, 11, 16, 21].includes(trCalc)) {
    ringColorName.innerHTML = "Blue";
    ringBlock.setAttribute("style", "background-color: #426ffe; color: white;");
}
if ([2, 7, 12, 17, 22].includes(trCalc)) {
    ringColorName.innerHTML = "Orange";
    ringBlock.setAttribute("style", "background-color: #f27f42; color: #2f2f2f;");
}
if ([3, 8, 13, 18, 23].includes(trCalc)) {
    ringColorName.innerHTML = "Green";
    ringBlock.setAttribute("style", "background-color: #25d366; color: white;");
}
if ([4, 9, 14, 19, 24].includes(trCalc)) {
    ringColorName.innerHTML = "Brown";
    ringBlock.setAttribute("style", "background-color: #917260; color: #2f2f2f;");
}
if ([5, 10, 15, 20 ].includes(trCalc) || num % 25 == 0 && num > 0 ) {
    ringColorName.innerHTML = "Slate";
    ringBlock.setAttribute("style", "background-color: grey; color: #2f2f2f;");
}

Any help is appreciated!

Optimization and minification are really two different things. look at this site for minification http://closure-compiler.appspot.com/home. — Thomas Valadez, Oct 30 '17 at 03:51
Can you provide some context on what the code does (or is supposed to do), and if there's anything in particular that you'd like to optimize. Can you also post any attempts that you've made to do this? — roelofs, Oct 30 '17 at 03:54
@roelofs the OP sensed correctly that there's a pattern here and he's using more `if` statements than necessary. — Andy, Oct 30 '17 at 06:40

Thomas Valadez · Answer 1 · 2017-10-30T22:20:12.387

Optimization and minification are really two different things. look at this site for minification http://closure-compiler.appspot.com/home.

As far as optimization there are a couple of things you could do.

You could look at using one function to change your styling. which would clean up your code and make it more readable. Something like:

changeStyles(innerColor,bgColor,color){
  ringColorName.innerHTML = innerColor;
  ringBlock.setAttribute("style", "background-color: "+bgColor+"; color: "+color+";") 
}

Then your if statements would look something like this:

if ([1, 6, 11, 16, 21].includes(trCalc)) {
   changeStyles("White","#f2f2f2","#2f2f2f");
}

You could look at the modulo operator. For the last five if statements it looks like you could do foo = trCalc % 5 followed by a switch(foo). Might look something like this.

foo = trCalc % 5
switch(foo) {
    case 0:
        changeStyles("White","#f2f2f2","#2f2f2f");
        break;
    case 1:
        changeStyles("White","#f2f2f2","#2f2f2f");
        break;
    case 2:
        changeStyles("White","#f2f2f2","#2f2f2f");
        break;
    case 3:
        changeStyles("White","#f2f2f2","#2f2f2f");
        break;
    case 4:
        changeStyles("White","#f2f2f2","#2f2f2f");
        break;
}

Of course, you would have plug the appropriate colors into the function changeColors. Hopefully that gives you some ideas.

Andy · Answer 2 · 2017-10-30T06:34:52.100

Here is how I would optimize it:

const tipColorNames = ['White', 'Red', 'Black', 'Yellow', 'Violet'];
const tipStyles = [
  "background-color: #f2f2f2; color: #2f2f2f;",
  "background-color: #e24341; color: white;",
  "background-color: #2f2f2f; color: white;",
  "background-color: #f4ca37; color: #2f2f2f;",
  "background-color: #844ae0; color: white;",
];
const ringColorNames = ['Blue', 'Orange', 'Green', 'Brown', 'Slate'];
const ringStyles = [
  "background-color: #426ffe; color: white;",
  "background-color: #f27f42; color: #2f2f2f;",
  "background-color: #25d366; color: white;",
  "background-color: #917260; color: #2f2f2f;",
  "background-color: grey; color: #2f2f2f;",
];

const divisor = 5;
const numColors = tipColorNames.length;

function calculation() {
  var num = document.getElementById("input").value;
  var trCalc = num % (divisor * numColors);

  var tipColorName = document.getElementById("tipColorName");
  var ringColorName = document.getElementById("ringColorName");

  var tipBlock = document.getElementById("tipBlock");
  var ringBlock = document.getElementById("ringBlock");

  const tipColorIndex = trCalc === 0 && num > 0
    ? numColors - 1
    : (trCalc - 1) / divisor;

  tipColorName.innerHTML = tipColorNames[tipColorIndex];
  tipBlock.setAttribute('style', tipStyles[tipColorIndex]);

  const ringColorIndex = (num - 1) % numColors;
  ringColorName.innerHTML = ringColorNames[ringColorIndex];
  ringBlock.setAttribute('style', ringStyles[ringColorIndex]);
}

Optimizing code to map numbers 1 - 25 uniformly to 5 colors

2 Answers2