numbers to words - criticize my code
Mar 27, 2013 at 12:56pm UTC
Hello,
our teacher has given us the task of writing a program that converts numbers to words. Here is my approach:
1#include <iostream>
#include <string>
int modulo (int number);
using namespace std;
int number;
int digits_1;
int digits_2;
int digits_3;
int modulo (int number)
{
int z;
int digit1;
int digit2;
int digit3;
digit1 = number%10;
number = (number/10);
digit2 = number%10;
number = (number/10);
digit3 = number%10;
number = (number/10);
z = (digit1+(10*digit2)+(100*digit3));
return z;
}
int translation_digits (int x)
{
int lasttwodigits = x%100;
int firstdigit = ((x-lasttwodigits)/100);
// output first digit
if (firstdigit == 1)
{
cout << "one" ;
}
else if (firstdigit == 2)
{
cout << "two" ;
}
else if (firstdigit == 3)
{
cout << "three" ;
}
else if (firstdigit == 4)
{
cout << "four" ;
}
else if (firstdigit == 5)
{
cout << "five" ;
}
else if (firstdigit == 6)
{
cout << "six" ;
}
else if (firstdigit == 7)
{
cout << "seven" ;
}
else if (firstdigit == 8)
{
cout << "eight" ;
}
else if (firstdigit == 9)
{
cout << "nine" ;
}
if (firstdigit != 0)
{
cout << "hundred" ;
}
//nächste zwei digits auslesen
if (lasttwodigits == 1)
{
cout << "one" ;
}
else if (lasttwodigits == 2)
{
cout << "two" ;
}
else if (lasttwodigits == 3)
{
cout << "three" ;
}
else if (lasttwodigits == 4)
{
cout << "four" ;
}
else if (lasttwodigits == 5)
{
cout << "five" ;
}
else if (lasttwodigits == 6)
{
cout << " six" ;
}
else if (lasttwodigits == 7)
{
cout << "seven" ;
}
else if (lasttwodigits == 8)
{
cout << "eight" ;
}
else if (lasttwodigits == 9)
{
cout << "nine" ;
}
else if (lasttwodigits == 10)
{
cout << "ten" ;
}
else if (lasttwodigits == 11)
{
cout << "eleven" ;
}
else if (lasttwodigits == 12)
{
cout << "twelve" ;
}
else if (lasttwodigits == 13)
{
cout << "thirteen" ;
}
else if (lasttwodigits == 14)
{
cout << "fourteen" ;
}
else if (lasttwodigits == 15)
{
cout << "fifteen" ;
}
else if (lasttwodigits == 16)
{
cout << "sixteen" ;
}
else if (lasttwodigits == 17)
{
cout << "seventeen" ;
}
else if (lasttwodigits == 18)
{
cout << "eighteen" ;
}
else if (lasttwodigits == 19)
{
cout << "nineteen" ;
}
else // Zerlegung der zwei digits
{
int lastdigit = lasttwodigits%10;
int middigit = ((lasttwodigits-lastdigit)/10);
//middigit
if (middigit == 2)
{
cout << "twenty" ;
}
else if (middigit == 3)
{
cout << "thirty" ;
}
else if (middigit == 4)
{
cout << "fourty" ;
}
else if (middigit == 5)
{
cout << "fifty" ;
}
else if (middigit == 6)
{
cout << "sixty" ;
}
else if (middigit== 7)
{
cout << "seventy" ;
}
else if (middigit == 8)
{
cout << "eighty" ;
}
else if (middigit == 9)
{
cout << "ninety" ;
}
//lastdigit
if (lastdigit == 1)
{
cout << "one" ;
}
else if (lastdigit == 2)
{
cout << "two" ;
}
else if (lastdigit == 3)
{
cout << "three" ;
}
else if (lastdigit == 4)
{
cout << "four" ;
}
else if (lastdigit == 5)
{
cout << "five" ;
}
else if (lastdigit == 6)
{
cout << "six" ;
}
else if (lastdigit == 7)
{
cout << "seven" ;
}
else if (lastdigit == 8)
{
cout << "eight" ;
}
else if (lastdigit == 9)
{
cout << "nine" ;
}
}
}
int main()
{
while (1)
{
// User input;
cout << endl;
cout << "Enter a number between 1 - 999 999 999!" << endl;
cin >> number;
// check for zero case
if (number == 0)
{
cout << "You entered zero." ;
}
//separate into blocks of 3, calling the modulo function
digits_1 = modulo(number);
if (number > 0)
{
// number = ((number - digits_1)/1000);
digits_2 = modulo((number - digits_1)/1000);
if (number > 0)
{
// number = ((number - digits_2)/1000);
digits_3 = modulo( ( ( (number - digits_1)/1000) - digits_2)/1000);
}
}
// calling the translating function & output
if (number > 999999)
{
translation_digits (digits_3);
cout << " million " ;
}
if (number > 999)
{
translation_digits (digits_2);
cout << "thousand " ;
}
if (number > 0)
{
translation_digits (digits_1);
}
}
}
Now this code looks very messy and i'm looking for shortcuts, ie loops to shorten the code. Note: we didnt learn about arrays yet.
Thanks in advance.
Mar 27, 2013 at 1:48pm UTC
I'd trade your large if-else statements for switch statements, also called case statements. I'm not sure if you've learned about these yet, but here you go.
1switch (lastdigit) {
case 1:
cout << "one" ;
break ;
case 2:
cout << "two" ;
break ;
default :
break ;
}
The code above says, "In the case that last digit is equal to 1, print 'one' and exit the switch statement. In the case that lastdigit is equal to 2, print 'two' and exit the switch statement. In the case that the value of lastdigit does not meet any of the conditions above, exit the switch statement." You might not always need default, but it is good practice to throw it in there with some error code to tell the user that something went wrong.
Best of Luck,
--NolanNJW
Mar 27, 2013 at 2:15pm UTC
Check out this change I made to your translation_digits function. I reduced the amount of whitespace and took advantage of the fact that most of your blocks after your if statements were single lines of code:
1int translation_digits (int x)
{
int lasttwodigits = x%100;
int firstdigit = ((x-lasttwodigits)/100);
// output first digit
if (firstdigit == 1) cout << "one" ;
else if (firstdigit == 2) cout << "two" ;
else if (firstdigit == 3) cout << "three" ;
else if (firstdigit == 4) cout << "four" ;
else if (firstdigit == 5) cout << "five" ;
else if (firstdigit == 6) cout << "six" ;
else if (firstdigit == 7) cout << "seven" ;
else if (firstdigit == 8) cout << "eight" ;
else if (firstdigit == 9) cout << "nine" ;
if (firstdigit != 0) cout << "hundred" ;
//nächste zwei digits auslesen
if (lasttwodigits == 1)cout << "one" ;
else if (lasttwodigits == 2) cout << "two" ;
else if (lasttwodigits == 3) cout << "three" ;
else if (lasttwodigits == 4) cout << "four" ;
else if (lasttwodigits == 5) cout << "five" ;
else if (lasttwodigits == 6) cout << " six" ;
else if (lasttwodigits == 7) cout << "seven" ;
else if (lasttwodigits == 8) cout << "eight" ;
else if (lasttwodigits == 9) cout << "nine" ;
else if (lasttwodigits == 10) cout << "ten" ;
else if (lasttwodigits == 11) cout << "eleven" ;
else if (lasttwodigits == 12) cout << "twelve" ;
else if (lasttwodigits == 13) cout << "thirteen" ;
else if (lasttwodigits == 14) cout << "fourteen" ;
else if (lasttwodigits == 15) cout << "fifteen" ;
else if (lasttwodigits == 16) cout << "sixteen" ;
else if (lasttwodigits == 17) cout << "seventeen" ;
else if (lasttwodigits == 18) cout << "eighteen" ;
else if (lasttwodigits == 19) cout << "nineteen" ;
else // Zerlegung der zwei digits
{
int lastdigit = lasttwodigits%10;
int middigit = ((lasttwodigits-lastdigit)/10);
//middigit
if (middigit == 2) cout << "twenty" ;
else if (middigit == 3) cout << "thirty" ;
else if (middigit == 4) cout << "fourty" ;
else if (middigit == 5) cout << "fifty" ;
else if (middigit == 6) cout << "sixty" ;
else if (middigit == 7) cout << "seventy" ;
else if (middigit == 8) cout << "eighty" ;
else if (middigit == 9) cout << "ninety" ;
//lastdigit
if (lastdigit == 1) cout << "one" ;
else if (lastdigit == 2) cout << "two" ;
else if (lastdigit == 3) cout << "three" ;
else if (lastdigit == 4) cout << "four" ;
else if (lastdigit == 5) cout << "five" ;
else if (lastdigit == 6) cout << "six" ;
else if (lastdigit == 7) cout << "seven" ;
else if (lastdigit == 8) cout << "eight" ;
else if (lastdigit == 9) cout << "nine" ;
}
}
I believe this makes it much easier to read. I haven't checked your logic to see if there are errors, but this code should be functionally equivalent to the function you posted originally.
Mar 27, 2013 at 6:38pm UTC
@Nolan: We didnt get around to switch/case stuff yet, i believe thats up next.
Topic archived. No new replies allowed.
