8
\$\begingroup\$

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 < 20) {
    line = ones_teens[n];
  } else if (n < 100) {
    line = tens[(n / 10) | 0];
    if (n % 10) {
      line += "-" + ones_teens[n % 10];
    }
  } else {
    line = ones_teens[(n / 100) | 0] + " hundred " + toWords(n % 100);
  }

  return line.trim();
}

function chunkify(n) {
  var numbers = [];
  while (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(", "));
\$\endgroup\$
  • \$\begingroup\$ Related question \$\endgroup\$ – Flambino May 15 '15 at 23:23
  • \$\begingroup\$ @Flambino: I almost missed your comment because it looked like some random link. thanks. \$\endgroup\$ – Harish May 16 '15 at 7:17
  • \$\begingroup\$ @rolfl : I was reading an blog on codereview.stackexchange site and it supported to post an answer if there was any improvement to original code. I made some improvement by changing the approach to purely use functions. So I decided to post it but it was deleted by you. Why do you think it should be deleted? What should I need to do if I want to request feedback for my original code and new code? (both code does same thing). The reason I seek feedback for both codes is that it could help me understand the pros and cons of certain approach. Thank you. \$\endgroup\$ – Harish May 16 '15 at 14:17
  • \$\begingroup\$ @Harish Your answer was removed for a couple reasons. First, you did not provide a review, and second, you said "Please provide your feedback on my updated code." A self-answer would have perfectly acceptable if you provided it in the form of a review, specifying how your new code was better than your old code; however, you just posted your new code with a comment that it is your updated code. Also, we do not write reviews on answers, but on questions. Because there were no answers to this question, rolfl posted your new code to your question, instead of directing you to post another question. \$\endgroup\$ – 23fc9a62-56de-47fb-97b4-737890 Jun 4 '15 at 4:00
7
\$\begingroup\$

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.

\$\endgroup\$
  • 3
    \$\begingroup\$ || and | are not functionally identical! The former is a logical OR, and the latter is a bitwise OR. The trick OP is using, (n / 100) | 0, is a pseudo-shorthand for Math.floor due to some quirks with bitwise operations in JavaScript (namely that they only produce integers, thus flooring a float). Which is also why | is not at all the same as || \$\endgroup\$ – Flambino Jun 7 '15 at 19:21
  • \$\begingroup\$ @Flambino ...Oooh, that makes sense. I'll remove that point, then. I forgot it's not just a logical operator. Fixed! \$\endgroup\$ – Nic Hartley Jun 7 '15 at 19:22
  • \$\begingroup\$ It's never been a boolean comparator, though... For one, it doesn't compare anything, and secondly, it treats everything as a 32-bit integer. It knows nothing about actual booleans. false | true produces 1 because of a bunch of type coercion; 4.23 | true produces 5 for the same reasons. \$\endgroup\$ – Flambino Jun 7 '15 at 19:28
  • \$\begingroup\$ @Flambino ...Huh. I didn't know that. Either way, fixed. Thanks for pointing it out. \$\endgroup\$ – Nic Hartley Jun 7 '15 at 19:29
  • \$\begingroup\$ Thanks @QPaysTaxes. The reason I split the functionalities is to utilize functional programming technique, so that I could reuse the same function. For example: Assume if my methods were part of huge module, and chunkify(), I could use it to achieve splitting numbers into groups of 3 at other parts of the program. Also, since I intended to use higher order functions you see the functionalities separated. \$\endgroup\$ – Harish Jun 8 '15 at 6:27

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.