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

Converts number in text format function code

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

Problem

I have created a function which converts numbers into text format and successfully converted up to 6 digit numbers. However, I want to know if I can make any modifications to make the code shorter.



$(document).ready(function() {
var onesVal = ['Zero', 'One', 'Two', 'Three', 'Four', 'Five', 'Six', 'Seven', 'Eight', 'Nine'];
var twoVal = ['Ten', 'Eleven', 'Twelve', 'Thirteen', 'Fourteen', 'Fifteen', 'Sixteen', 'Seventeen', 'Eighteen', 'Ninteen'];
var tensVal = ["", "", 'Twenty', 'Thirty', 'Fourty', 'Fifty', 'Sixty', 'Seventy', 'Eighty', 'Ninty'];
var powersVal = ['Hundred', 'Thousand', 'Lakh'];

$('input').on('input propertychange', function() {
if ($(this).val() == '') {
$('p').text('');
} else {
//var textVal = $(this).val();
var textVal = parseInt($(this).val());
if (textVal

Enter Number :

`

Solution

A few thoughts:

  • The whole logic seems much too hard-coded. Think about the problem, which is that you basically need to take the string representation of the number and grab a group of the last three characters off the string at a time to process, the whole time tracking the number of "groups" that have been processed to be able to add "thousand", "million", etc. to it. Think about breaking apart your function along those lines instead of having a bunch of for*Value functions.



  • You should only have one point in our code where you are writing to the target p element after the word string has been built. There is no reason to repeat ('p').text(*); throughout your code.



  • Consider encapsulating all your conversion logic under one function such that your code in the event handler can be greatly simplified and you could re-use this function elsewhere.



For example:

$('input').on('input propertychange', function() {
  $('p').text(
    // numberToWords() could be included from another file
    numberToWords($(this).val())
  );
}


  • In general, I would recommend you getting in the habit of using exact comparisons (=== and !==) instead of loose comparisons as default behavior. This will make your code less fragile to unexpected truthy/falsey behavior. Use loose comparisons only where you have a specific use case to do so.



  • I don't think "zero" needs to be in your array, as this is an edge case that should be handled upfront before the main logic.

Code Snippets

$('input').on('input propertychange', function() {
  $('p').text(
    // numberToWords() could be included from another file
    numberToWords($(this).val())
  );
}

Context

StackExchange Code Review Q#160309, answer score: 4

Revisions (0)

No revisions yet.