Converting numbers to words - JavaScript

Question

I recently worked on this problem that converts numbers to words. I was wondering if there is a better way to do it than mine, And I will really appreciate any reviews to my version of the solution and on how to make it better

/**
 * Converts a given number into words
 * @param {integer} , num, the number to be converted into words
 * @return {string} , the coverted number in words
 */
function numToEng(num){
  let numInWords = '';  

  // will be using this as a number that will be reduced by 1000
  let tempNum = num; 

  const wordsForConstruction = {
    0: 'zero',
    1: 'one',
    2: 'two',
    3: 'three',
    4: 'four',
    5: 'five',
    6: 'six',
    7: 'seven',
    8: 'eight',
    9: 'nine',
    10: 'ten',
    11: 'elleven',
    12: 'twelve',
    13: 'thirteen',
    14: 'fourteen',
    15: 'fifteen',
    16: 'sixteen',
    17: 'seventeen',
    18: 'eighteen',
    19: 'nineteen',
    20: 'twenty',
    30: 'thirty',
    40: 'fourty',
    50: 'fifty',
    60: 'sixty',
    70: 'seventy',
    80: 'eighty',
    90: 'ninety',
  }

  const wordsToAdd = [
    'thousand',
    'million',
    'trillion',
  ]

  if(num >= 0 && num <= 19){
    return `${wordsForConstruction[num]}`;
  }

  let hundreds;
  let tens;
  let ones;

  for(let i = 0; tempNum; i++){

    hundreds = tempNum % 1000;
    tempNum = (tempNum-hundreds)/1000;

    // construct words for hundred only if it is not a zero
    if(hundreds){

      // If the loop runs the second time, this will add the words
      // thousand, million, trillion, and so on, depending on the num
      // of words in the wordsToAdd[] Array
      if(i){
        numInWords = wordsToAdd[i-1] + " " + numInWords;
      }

      tens = hundreds % 100;
      hundreds = (hundreds - tens)/100;

      // construct words for tens only if it is not a zero
      if(tens){

        // is tens is a property of the wordsforconstruction then
        // just pull the word from the object
        if(wordsForConstruction.hasOwnProperty(tens)){
          numInWords = wordsForConstruction[tens] + " " + numInWords;
        }else{
          ones = tens%10;
          tens = tens-ones;
          if(ones){
            numInWords = wordsForConstruction[ones] + " " + numInWords;
          }
          numInWords = wordsForConstruction[tens] + " " + numInWords;
        }
      }

      if(hundreds){
        numInWords = wordsForConstruction[hundreds] + " hundred " + numInWords;
      }
    }
  }

  return numInWords;
}

/**
 * Initiates the program and runs the test cases
 */
function main(){
  const testCases = [
    0,
    101,
    100234,
    909,
    1000000,
    1000,
    9999999,
    90909090,
  ]

  testCases.map(tCase => {
    console.log();
    console.log(`numToEng(${tCase}) ➞ ${numToEng(tCase)}`);
  })
}

main();

Also if you find any bugs or issues with the solution please feel free to reply to this thanks in advance.

Answer 1

I like how you've reduced all of the words to lists, that's the right start.

Let's get into it:

  for(let i = 0; tempNum; i++){

I have no idea what you've doing here, I assume when tempNum===0 then the loop stops?

Don't do this, it's confusing as hell.

I'm going to suggest that you rewrite the entire thing as a recursive function or a reducing function, but if you insist on using a loop, then just make it a while(true) loop and add a return or a break statement to it to exit it.

You have a lot of branching logic:

  for(let i = 0; tempNum; i++){
    if(hundreds){
      if(i){
      }
      if(tens){
        if(wordsForConstruction.hasOwnProperty(tens)){

        }else{
          if(ones){
          }
        }
      }

      if(hundreds){
      }
    }
  }

The term to think about here is cyclomatic complexity keeping track of which branch the code is currently in involves holding a lot of information in your head, and then you have to run through it again for the next number etc. If you code is looking like a lot of these nested if statements, that's a code smell, and you should be doing something else.

The real issue you have, is that you are manually writing the code to do the 'detect how many hundreds, thousands, millions', instead you you could abstract this to a map.

What I would do is:

const powerMap = {
  "million": 1000000, 
  "thousand": 1000, 
  "hundred" : 100,  
  "rest": 1, 
}; //And ofcourse you can extend this to billions, trillions, etc. 

And then you can create a general abstraction to to count how many of each there are:

const powerMap = {
  "million": 1000000, 
  "thousand": 1000, 
  "hundred" : 100,  
  "rest": 1, 
}; //And ofcourse you can extend this to billions, trillions, etc. 


function getUnitCounts(value) {
    const data = Object.entries(powerMap).reduce(({value, result}, [unitName, power]) => {
        const nCount = Math.floor(value/power); 
        const nRemainder = value%power; 
        
        return {
            value: nRemainder, 
            result: {
                ...result, 
                [unitName]: nCount
            }
        }           
    
    }, {
         value, 
         result: {}
    }); 
    
    return data.result;        
    
}


console.log(getUnitCounts(20020000)); 
console.log(getUnitCounts(1100)); 
console.log(getUnitCounts(201110)); 

Now - if you're not familar with spread syntax and Array.reduce this might seem confusing. But notice how I don't use a single if statement - all I'm doing is looping over a list and returning an accumulator. Once you've familiar with the odd syntax, this is easier to keep in your head.

From the answer I've given here, it would now be a matter of converting the count of each unit into the words, and then also applying the words for 0-100 like you have.