3
\$\begingroup\$

I'm working with an API that for some strange reason returns star ratings as words, like FIVE,FOUR,THREE,TWO,ONE. As far as I can see, there's no way to just get the numerical rating, so I have to locally convert these into numbers.

I went with a switch statement, but honestly whenever I see myself using switch cases I feel like there must be a better way. It's obviously very readable and clear, which is good, but I can't help but wonder, there must be a better way of dealing with something like this right?

Current code looks like this

function wordToNumber(word) {
  switch (word) {
    case "FIVE":
      return 5;
    case "FOUR":
      return 4;
    case "THREE":
      return 3;
    case "TWO":
      return 2;
    case "ONE":
      return 1;
    default:
      return "N/A";
  }
}

I'm probably overthinking this honestly, but I just always feel a bit unsure when I reach for switch statements.

EDIT:

Some of the answers rightfully point out that I'm returning integers in the cases, but then in the default I'm returning a string. In my particular usecase it's fine, since this function is returning it directly into a render function that will convert the ints into strings anyway, and the numbers get used nowhere else either.

As such though, I've reformatted it to the below instead, and I instead check whether it returned -1 and then print out a different string based on that instead.

default:
  return -1;
\$\endgroup\$
6
\$\begingroup\$

Complexity and performance

There are many ways to write this type of function and the best is determined by the number of input states. In your example you have 6 different input states 1-5 and NaN.

Complexity

The least number of steps to parse the string is 1 and the worst requires 6 steps. The average time, assuming equal distribution of input, is (1 + 2 + 3 + 4 + 5 + 6) / 6 = 21/6 = 3.5

However you can reduce the number of steps required using a hash map.

The input word is first converted to a hash value (considered as one step and doen by the JS engine). The hash value is then used to locate the return value. The examples below have an average number of steps = 1

const STR_TO_NUM = {one: 1,  two: 2, three: 3, four: 4, five: 5};
const wordToNumber = word => Number(STR_TO_NUM[word.toLowerCase()]);

or

const STR_TO_NUM = new Map([["one", 1], ["two", 2], ["three", 3], ["four", 4], ["five": 5]]);
const wordToNumber = word => Number(STR_TO_NUM.get(word.toLowerCase()));

Note: Assumes word is a string

Performance

The above function requires only 1 step, however it does not mean it will be quicker. The step of converting to a hash can take much more time than the step that compares a string to another string. In your case where there are only 6 states it will be quicker to use the original switch, or conditional search.

function wordToNumber(word) {   /* Note assumes word is a string */
  switch (word.toUpperCase()) {
    case "FIVE":  return 5;
    case "FOUR":  return 4;
    case "THREE": return 3;
    case "TWO":   return 2;
    case "ONE":   return 1;
    default:      return NaN;
  }
}

or

function wordToNumber(word) {   /* Note assumes word is a string */
  word = word.toUpperCase();
  if (word === "FIVE")  { return 5; }
  if (word === "FOUR")  { return 4; }
  if (word === "THREE") { return 3; }
  if (word === "TWO")   { return 2; }
  if (word === "ONE")   { return 1; }
  return NaN;
}

As the number of input states grows the cost of finding the hash value becomes negligible compared to the many conditions to test. If you had 20 input states it would be quicker to use a hash map.

Aiming for performance

As the function is not a generic solution. ie the number of states is constant you should aim for the best performance, not the least complexity

You could exploit the uniqueness and similarity of the inputs

const wordToNumber = word => {   /* Note assumes word is a string */
    const w = word.toLowerCase(word);
    if (w[0] === "t") { return w[1] === "w" ? 2 : 3; }
    if (w[0] === "f") { return w[1] === "o" ? 4 : 5; }    ​
    ​return w[0] === "o" ? 1 : NaN;
}

Now the average number of steps is (3 + 2 + 2 + 3 + 3 + 3) / 6 = 16 / 6 = 2.666...

Note that if the input range was 1 - 1000 the above approach will be optimal as you can break the string into parts and parse the parts, rather than have 1000 items in a hash map or 1000 conditional statements.

If you know the odds of each input you can also change the order of the steps to favor the most likely inputs. If high values are more likely (eg "five" is 10 times more likely than "one") you would write the function as

const wordToNumber = word => {   /* Note assumes word is a string */
    const w = word.toLowerCase(word);
    if (w[0] === "f") { return w[1] === "o" ? 4 : 5; }    ​
    if (w[0] === "t") { return w[1] === "w" ? 2 : 3; }
    ​return w[0] === "o" ? 1 : NaN;
}

Note that there are more optimizations possible but for such a small input set it may not be worth the effort unless you need to call the function millions of times a second.

\$\endgroup\$
3
  • \$\begingroup\$ Amazing. This should be the accepted answer. How about if we switch case on word[1] since it is distinct in every word in the set? \$\endgroup\$
    – s.alem
    Nov 12 '21 at 18:49
  • 2
    \$\begingroup\$ @s.alem If you switch on the second character it would be best to switch on the character code as JS handles numbers far quicker than characters eg switch(word.charCodeAt(1)) { case 110: return 1; where 110 is char code for n. If you know that the string only contains a-zA-Z chars you can combine the case conversion by adding the 6th bit to convert to lower. Eg switch(word.charCodeAt(1) | 0b100000) { case 110: return 1; /*etc*/ Will convert A-Z to lower case character code so that case 110 will be true for "N" and "n" \$\endgroup\$
    – Blindman67
    Nov 12 '21 at 19:26
  • \$\begingroup\$ Thanks for explaining it. Maybe you can add this comment to the answer. Just out of pure curiosity, while staying in JS, what is the max optimization you can think for this case? \$\endgroup\$
    – s.alem
    Nov 12 '21 at 21:36
3
\$\begingroup\$

You could use a map or object, but it would be just another way of doing it and I wouldn't call it better tbh.

One thing you can change is the default block. All the other cases return a number while default returns a string. Depending on the use case, you can either return a value like -1, NaN, undefined, or even throw an exception throw 'Not a valid value!'.

Also you can make it easily case agnostic:

function wordToNumber(word) {
  upperCaseWord = word?.toUpperCase();
  switch (upperCaseWord) {
    case "FIVE":
      return 5;
    case "FOUR":
      return 4;
    case "THREE":
      return 3;
    case "TWO":
      return 2;
    case "ONE":
      return 1;
    default:
      return NaN;
  }
}
\$\endgroup\$
1
  • \$\begingroup\$ I''m using this in a Frontend app that's displaying the score to the user. As such, when there's no Score (the API returns a null field when there's no score), I just return N/A to indicate it's not applicable. Agreed that I should rethink how I handle that though, it looks fine in context but it's not the best way of handling things \$\endgroup\$
    – Sensanaty
    Nov 12 '21 at 15:58
2
\$\begingroup\$

I'm not a Javascript programmer, but it looks strange that we return a different type of result for the default case. Can't we return a number (e.g. 0)? Or a NaN? Or perhaps even throw an exception.

I guess that without more context of how the function is used, it's hard to say what the default return should be.

\$\endgroup\$
1
  • \$\begingroup\$ Refer to my comment on @s.alem's answer, but basically this function directly returns the value into text that is rendered to the user in their browser. As suich, when there's no score I want to let them know that particular entry isn't applicable, so I return a string that's saying "Yeah this one doesn't exist", the scores aren't used anywhere else so they could be strings as well. \$\endgroup\$
    – Sensanaty
    Nov 12 '21 at 16:02
0
\$\begingroup\$

Super short review,

  • as others have said, a function should try to be consistent in the type of data it returns
  • your case statement can be replaced with a clever data structure
  • I prefer <verb><subject> for function names, so convertWordToNumber

Personally, after seeing you now return -1 for non found values, I would write this as

function convertStarCount(starCount) {
  const integers = ["ZERO", "ONE", "TWO", "THREE", "FOUR", "FIVE"];
  return integers.indexOf(starCount);
}

I assumed something can get zero stars ;)

And I trusted that the API you use will always send upper case strings.

\$\endgroup\$

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.