patternjavaModerate
Numbers to words converter
Viewed 0 times
numberswordsconverter
Problem
Please review my code:
```
import java.lang.*;
import java.util.*;
import java.io.*;
class NumberToWords
{
static public boolean HelperConvertNumberToText(int num, String[] result)
{
String [] strones = {
"One", "Two", "Three", "Four", "Five", "Six", "Seven", "Eight",
"Nine", "Ten", "Eleven", "Twelve", "Thirteen", "Fourteen",
"Fifteen", "Sixteen", "Seventeen", "Eighteen", "Nineteen",
};
String [] strtens = {
"Ten", "Twenty", "Thirty", "Fourty", "Fifty", "Sixty",
"Seventy", "Eighty", "Ninety", "Hundred"
};
result[0] = "";
int single, tens, hundreds;
if(num > 1000)
return false;
hundreds = num / 100;
num = num - hundreds * 100;
if( num 0)
{
result[0] += strones[hundreds-1];
result[0] += " Hundred ";
}
if(tens > 0)
{
result[0] += strtens[tens - 1];
result[0] += " ";
}
if(single > 0)
{
result[0] += strones[single - 1];
result[0] += " ";
}
return true;
}
static public boolean ConvertNumberToText(int num, String[] result)
{
String tempString[] = new String[1];
tempString[0] = "";
int thousands;
int temp;
result[0] = "";
if(num 100000)
{
System.out.println(num + " \tNot Supported");
return false;
}
if( num == 0)
{
System.out.println(num + " \tZero");
return false;
}
if(num < 1000)
{
HelperConvertNumberToText(num, tempString);
result[0] += tempString[0];
}
else
{
thousands = num / 1000;
temp = num - thousands * 1000;
HelperConvertNumberToText(thousands, tempString);
result[0] += tempString[0];
result[0] += "Thousand ";
HelperConvertNumberToText(temp, tempString);
result[0] += tempString[0];
}
```
import java.lang.*;
import java.util.*;
import java.io.*;
class NumberToWords
{
static public boolean HelperConvertNumberToText(int num, String[] result)
{
String [] strones = {
"One", "Two", "Three", "Four", "Five", "Six", "Seven", "Eight",
"Nine", "Ten", "Eleven", "Twelve", "Thirteen", "Fourteen",
"Fifteen", "Sixteen", "Seventeen", "Eighteen", "Nineteen",
};
String [] strtens = {
"Ten", "Twenty", "Thirty", "Fourty", "Fifty", "Sixty",
"Seventy", "Eighty", "Ninety", "Hundred"
};
result[0] = "";
int single, tens, hundreds;
if(num > 1000)
return false;
hundreds = num / 100;
num = num - hundreds * 100;
if( num 0)
{
result[0] += strones[hundreds-1];
result[0] += " Hundred ";
}
if(tens > 0)
{
result[0] += strtens[tens - 1];
result[0] += " ";
}
if(single > 0)
{
result[0] += strones[single - 1];
result[0] += " ";
}
return true;
}
static public boolean ConvertNumberToText(int num, String[] result)
{
String tempString[] = new String[1];
tempString[0] = "";
int thousands;
int temp;
result[0] = "";
if(num 100000)
{
System.out.println(num + " \tNot Supported");
return false;
}
if( num == 0)
{
System.out.println(num + " \tZero");
return false;
}
if(num < 1000)
{
HelperConvertNumberToText(num, tempString);
result[0] += tempString[0];
}
else
{
thousands = num / 1000;
temp = num - thousands * 1000;
HelperConvertNumberToText(thousands, tempString);
result[0] += tempString[0];
result[0] += "Thousand ";
HelperConvertNumberToText(temp, tempString);
result[0] += tempString[0];
}
Solution
"Strone"? What's a "strone"? Oh… of course!
In seriousness, naming could be improved. They are obviously arrays of strings; the Hungarian prefix is unnecessary. Also, as constants, they should be declared
Helper functions should be
It is customary to put the
Using a single-element array as a parameter so that you can pass back the result as an out-parameter is very weird. Using a
In seriousness, naming could be improved. They are obviously arrays of strings; the Hungarian prefix is unnecessary. Also, as constants, they should be declared
static final and named with ALL_CAPS.public class NumberToWords {
private static final ONES = { "One", "Two", … };
private static final TENS = { "Ten", "Twenty", … };
…
}Helper functions should be
private.It is customary to put the
static keyword after the access modifier, as in public static void main(…). It is unconventional to say static public boolean ConvertNumberToText(…). Also, functions should be namedStartingWithLowerCase().Using a single-element array as a parameter so that you can pass back the result as an out-parameter is very weird. Using a
StringBuilder would be a bit more obvious, but still very odd. The right approach should be to return a String, and throw an exception if necessary — perhaps NumberFormatException. Ideally, your code should be able to handle every possible int input, making errors impossible.Code Snippets
public class NumberToWords {
private static final ONES = { "One", "Two", … };
private static final TENS = { "Ten", "Twenty", … };
…
}Context
StackExchange Code Review Q#60859, answer score: 11
Revisions (0)
No revisions yet.