5
\$\begingroup\$

I have an array of objects, reviews which I get from an API endpoint. This object looks like this:

[
  { name: "Facebook", count: 50 },
  { name: "Google", count: 43 },
  { name: "TripAdvisor", count: 67 },
  { name: "Other", count: 130 }
]

I have a chart.js Pie chart that I'm feeding this data to. I'm using the name for the labels and the count for the size of each slice of the pie chart. Now, due to how chart.js works, I have to supply some hexadecimal colors to use as background colors, and the order in which the background color is being defined matters if I want the labels to match the colors properly. For example, if the order of the labels is [A, B, C], an array of colors like [red, green, blue] means that A = red, B = green and C = blue. The problem is that the reviews array comes in a random order every single time from the API (I don't have control over the API and the data it gives me), so I can't just have a simple, predetermined array.

As far as I see, I have two options, I either sort the reviews array in some way, or I iterate over the data I get from the API and return an array based on some simple switch statements. I opted for the 2nd option, as it seems simpler for my usecase.

My current solution looks like this

const chartBackgroundColor = collection => {
  let colorArray = [];
  const itemArray = collection.map(data => data.name);
  itemArray.forEach(item => {
      switch (item) {
        case "TripAdvisor":
          colorArray.push("#00B98B");
          break;
        case "Google":
          colorArray.push("#FFBD00");
          break;
        case "Facebook":
          colorArray.push("#4B6DAA");
          break;
        default:
          colorArray.push("#D9D9D9");
          break;
      }
    }
  )
  return colorArray;
}

The method works fine, but I was just curious whether there was some more elegant way of handling this issue, and whether a switch case is appropriate in this situation rather than a if/ifel/else tree.

\$\endgroup\$
6
\$\begingroup\$

I would

  • Encapsulate the names and colors in a data structure
  • Use .map instead of .forEach

So something like this;

function chartBackgroundColor(collection){
  const nameColorMap = {
    "TripAdvisor": "#00B98B",
    "Google": "#FFBD00",
    "Facebook": "#4B6DAA",
  };
  return collection.map(data => nameColorMap[data.name] || "#D9D9D9");
}
\$\endgroup\$
  • \$\begingroup\$ Really nice one! Thanks for the help \$\endgroup\$ – Sensanaty Jan 11 at 14:48
  • \$\begingroup\$ Just as a small potential improvement, would it not be worthwhile declaring nameColorMap outside of the function? Means it could be used elsewhere (if exported) and won't be redeclared each time the function is run. I also believe the = sign should be removed on the return. Not sure if that's valid JS. \$\endgroup\$ – Dane Brouwer Jan 12 at 7:27
  • 1
    \$\begingroup\$ @DaneBrouwer if the map were to be used outside of this function -> Totally yes. Otherwise -> No. I imagine the speed gain would be minimal but the memory increase would be permanent. \$\endgroup\$ – konijn Jan 12 at 7:32

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.