HiveBrain v1.2.0
Get Started
← Back to all entries
snippetjavascriptMinor

Code to convert number to words

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
codewordsnumberconvert

Problem

I am trying to practice objects and functions (specifically closures & higher order functions) in javascript. There is no specific reason why I chose this program but it struck my mind to work out this one.

The below code is available at my Github and it converts the number to words.

Kindly provide your feedback and suggest which parts of the code can be improved or modified.

// Author       : Harish Narayanan
// Date         : 15 May 2015
// Description  : Code to convert number to words

var ones_teens = ['', 'one', 'two', 'three', 'four', 'five', 'six', 'seven', 'eight', 'nine', 'ten', 'eleven',
  'twelve', 'thirteen', 'fourteen', 'fifteen', 'sixteen', 'seventeen', 'eighteen', 'nineteen'
];
var tens = ['', '', 'twenty', 'thirty', 'forty', 'fifty', 'sixty', 'seventy', 'eighty', 'ninety'];
var scales = ['', 'thousand', 'million', 'billion', 'trillion', 'quadrillion', 'quintillion',
  'sextillion', 'septillion', 'octillion', 'nonillion', 'decillion'
];

function toWords(n) {
  var line;
  if (isNaN(n) || n > 999) {
    return null;
  }

  if (n == 0) {
    line = "";
  } else if (n  0) {
    numbers[numbers.length] = n % 1000;
    n = Math.floor(n / 1000);
  }
  return numbers;
}

function scalify(item, index) {
  if (item) {
    return item + " " + scales[index];
  }
  return "";
}

function notEmpty(item) {
  return !!item; // using double ! returns boolean value. !!item is not same as item.
}

console.log(chunkify(999999999999999).map(toWords).map(scalify).filter(notEmpty).reverse().join(", "));

Solution

Really not much to review, here. I think you did a pretty good job. There are a few points I'd like to make, though:

  • Indents in Javascript (at least as far as I've seen) are four spaces, not two.



  • When you're continuing a line, you indent twice; this applies to things like where you defined scales.



  • Use || in place of | wherever possible. Functionally identicaly, but it's a free little performance boost at no cost. Oops! turns out that's wrong. Thanks to @Flambino for pointing that out. I was just thinking in terms of logical operators, not bitwise ones. However, I would recommend using Math.floor instead of it; it's clearer.



  • Rather than using array[array.length] = ..., use array.push(...). Much cleaner.



  • You never seem to handle negative numbers. See below for how you could do that without changing lots of your code.



One big point: Your toWords function should do everything itself. You shouldn't have to call chunkify, then map, then map again, then filter, etc. etc. Wrap all that in a function and you'll be golden. Plus, that lets you do fancy stuff like handle negative numbers -- for example, this (should) do the trick:

function toWords(n) {
    var asString =  chunkify(n).map(toWords).map(scalify).filter(notEmpty).reverse().join(", ");
    if (n < 0) {
        asString = "negative " + asString;
    }
    return asString
}


Anyway, that's about it! Nicely written.

Code Snippets

function toWords(n) {
    var asString =  chunkify(n).map(toWords).map(scalify).filter(notEmpty).reverse().join(", ");
    if (n < 0) {
        asString = "negative " + asString;
    }
    return asString
}

Context

StackExchange Code Review Q#90838, answer score: 7

Revisions (0)

No revisions yet.