For my homework assignment I have to create a class which converts an integer
between 0 and 9999 into the written form. For example, 713 would be written as "seven hundred thirteen."
I wrote a few variations and I think I found the best approach. However, I was wondering if anyone had a moment to comment on my approach. Maybe point out subtle things I might not be taking advantage of. Things like that.
I'll list just the important excerpts from the .cpp
and .h
files.
The following are all static
members of the Numbers
class:
const char* Numbers::lessThan20[] = {
"zero", "one", "two", "three", "four", "five",
"six", "seven", "eight", "nine", "ten", "eleven",
"twelve", "thirteen", "fourteen", "fifteen", "sixteen", "seventeen",
"eighteen", "nineteen"
};
const char* Numbers::over19[] = {
"", "", "twenty", "thirty", "fourty", "fifty", "sixty", "seventy",
"eighty", "ninety"
};
const char* Numbers::hundred = "hundred";
const char* Numbers::thousand = "thousand";
And this is my function to convert the number into a string:
std::string Numbers::print() const
{
std::string text;
int whole, remainder;
remainder = this->number; // <- number is a member variable of the Numbers class
whole = remainder / 1000;
remainder %= 1000;
if (whole > 0)
text = text + lessThan20[whole] + " " + thousand + " ";
whole = remainder / 100;
remainder %= 100;
if (whole > 0)
text = text + lessThan20[whole] + " " + hundred + " ";
if (remainder > 19)
{
whole = remainder / 10;
remainder %= 10;
text = text + over19[whole];
if (remainder > 0)
text = text + "-" + lessThan20[remainder];
}
else if (remainder > 0)
text = text + lessThan20[remainder];
return text;
}
-
\$\begingroup\$ The algorithm is close to what I recall from my library for programming contests, thus I don't think there is a better algorithm for english. However, see Omnifarious' answer for extending this to million and so on. \$\endgroup\$Sjoerd– Sjoerd2011年12月14日 17:08:29 +00:00Commented Dec 14, 2011 at 17:08
4 Answers 4
Use STL Containers
Prefer using std::array
(if your compiler supports it or) std::vector
instead of raw arrays. Also, I would recommend using std::string
instead raw character pointers.
const std::array<std::string, 20> Numbers::lessThan20 = {
"zero", "one", "two", "three", "four", "five",
"six", "seven", "eight", "nine", "ten", "eleven",
"twelve", "thirteen", "fourteen", "fifteen", "sixteen", "seventeen",
"eighteen", "nineteen"
};
const std::array<std::string, 10> Numbers::over19 = {
"", "", "twenty", "thirty", "fourty", "fifty", "sixty", "seventy",
"eighty", "ninety"
};
const std::string Numbers::hundred = "hundred";
const std::string Numbers::thousand = "thousand";
Edge Cases
Currently there is no handling for edge cases. If the number is 0
it will return an empty string instead of "zero". Numbers greater than or equal to 20000 will cause an out of range access. Therefore, numbers too large need to be properly handled. These cases can checked at the beginning of the function.
A couple other minor points: I would remove the line int whole, remainder
and declare the variables when they are initialized. This will reduce the variables' scope and does not leave them initialized. Also, prefer text += ...
over text = text + ...
. It's more clear that you are appending and less typing. Lastly, I would recommend to always use curly braces even when there is only one statement.
Here is your function with my recommendations:
std::string Numbers::print() const
{
int remainder = this->number; // <- number is a member variable of the Numbers class
if(remainder == 0)
{
return lessThan20[0];
}
if(remainder >= 20000)
{
// handle this case
}
int whole = remainder / 1000;
remainder %= 1000;
std::string text;
if (whole > 0)
{
text += lessThan20[whole] + " " + thousand + " ";
}
whole = remainder / 100;
remainder %= 100;
if (whole > 0)
{
text = text + lessThan20[whole] + " " + hundred + " ";
}
if (remainder > 19)
{
whole = remainder / 10;
remainder %= 10;
text = text + over19[whole];
if (remainder > 0)
{
text = text + "-" + lessThan20[remainder];
}
}
else if (remainder > 0)
{
text = text + lessThan20[remainder];
}
return text;
}
-
1\$\begingroup\$ Use
boost::array
ifstd::array
is not supported. \$\endgroup\$user36– user362012年01月01日 16:17:16 +00:00Commented Jan 1, 2012 at 16:17
const char* Numbers::lessThan20[] = {
"zero", "one", "two", "three", "four", "five",
"six", "seven", "eight", "nine", "ten", "eleven",
"twelve", "thirteen", "fourteen", "fifteen", "sixteen", "seventeen",
"eighteen", "nineteen"
};
const char* Numbers::over19[] = {
"", "", "twenty", "thirty", "fourty", "fifty", "sixty", "seventy",
"eighty", "ninety"
};
Over 19 is a bit of a strange name for this array. Perhaps there is a better one
const char* Numbers::hundred = "hundred";
const char* Numbers::thousand = "thousand";
std::string Numbers::print() const
{
std::string text;
int whole, remainder;
remainder = this->number; // <- number is a member variable of the Numbers class
It would make more sense to me to pass the number as a parameter.
whole = remainder / 1000;
I'd call whole here thousands
to make the code easier to follow
remainder %= 1000;
if (whole > 0)
text = text + lessThan20[whole] + " " + thousand + " ";
Some teachers won't like you not putting braces around your statement. I'd also handle this differently. The way I see it, there is a parallel to how the numbers are handled. So I would do something like:
if( number >= 1000 )
return threeDigitNumber(number / 1000) + " thousand " + threeDigitNumber(number % 1000)
else
return threeDigitNumber(number)
I figure that would simply the code, it would also handle some larger cases then your code does.
whole = remainder / 100;
remainder %= 100;
Again I'd call this hundreds for clarity, not whole
if (whole > 0)
text = text + lessThan20[whole] + " " + hundred + " ";
if (remainder > 19)
{
whole = remainder / 10;
remainder %= 10;
text = text + over19[whole];
if (remainder > 0)
text = text + "-" + lessThan20[remainder];
}
else if (remainder > 0)
text = text + lessThan20[remainder];
I dislike jumping between braces and no brace. I'd put braces around the second one just for consistency.
return text;
}
The design of your algorithm is not very extensible. There is a system for handling this. For example 'one hundred fifty million sixty three thousand five hundred seventy six' could be done fairly simply if you re-arranged how your algorithm works just a little bit. As it is, you'd have to special case it all in.
#include <iostream>
#include <stdexcept>
namespace {
void less_than_100_to_english(::std::ostream &os, unsigned int num)
{
const char *needed_space = "";
static const char * const under_20[20] = {
"zero", "one", "two", "three", "four", "five",
"six", "seven", "eight", "nine", "ten", "eleven",
"twelve", "thirteen", "fourteen", "fifteen", "sixteen", "seventeen",
"eighteen", "nineteen" };
static const char * const tens[10] = {
"", "", "twenty", "thirty", "fourty", "fifty", "sixty", "seventy",
"eighty", "ninety" };
if (num >= 100) {
needed_space = " ";
os << under_20[num / 100] << " hundred";
}
num %= 100;
if ((num != 0) && (num < 20)) {
os << needed_space << under_20[num];
} else {
os << needed_space << tens[num / 10];
num %= 10;
if (num != 0) {
os << ' ' << under_20[num];
}
}
}
void number_to_english(::std::ostream &os, unsigned int num, unsigned int multiplier)
{
const char * const multipliers[] = {
"", "thousand", "million", "billion", "trillion", "quadrillion",
"quintillion", "sextillion", "septillion", "octillion" };
const char *needed_space = "";
if (multiplier >= (sizeof(multipliers) / sizeof(multipliers[0]))) {
throw ::std::overflow_error("Number too big to print in English.");
}
if (num >= 1000) {
number_to_english(os, num / 1000, multiplier + 1);
needed_space = " ";
}
const unsigned int part = num % 1000;
if (part != 0) {
os << needed_space;
less_than_100_to_english(os, part);
if (multiplier > 0) {
os << ' ' << multipliers[multiplier];
}
}
}
} // anonymous namespace
void number_to_english(::std::ostream &os, unsigned int num)
{
if (num == 0) {
os << "zero";
} else {
number_to_english(os, num, 0);
}
}
int main()
{
using ::std::cout;
number_to_english(cout, 1535201229u);
cout << '\n';
number_to_english(cout, 3115654019u);
cout << '\n';
number_to_english(cout, 0);
cout << '\n';
number_to_english(cout, 1);
cout << '\n';
return 0;
}
-
\$\begingroup\$ I like this approach. I'm gonna play with it. However, for the assignment I just needed to deal with 0-9999. I fixed the 0 issue which I overlooked. However, i'm going to try to implement this algorithm but tweak it for the specs of the assignment. \$\endgroup\$user9124– user91242011年12月13日 08:34:10 +00:00Commented Dec 13, 2011 at 8:34
I can't comment (not enough rep I guess), but one minor thing I noticed is that if the input number is evenly divisible by 100 or 1000, it will have a space at the end of the printed string. Example: 2000 -> "two thousand "
If that is even an issue, you can either:
- trim the whitespace from the string
- append a space to the end of
text
in every place you're adding to it and then remove the last character (the space) before returning it
-
\$\begingroup\$ Thanks for that. I think I can use a conditional statement when assembling the string to determine whether or not to add the " ". Was the rest of the logic easy to follow, clear, etc? \$\endgroup\$user9124– user91242011年12月12日 22:43:45 +00:00Commented Dec 12, 2011 at 22:43
-
\$\begingroup\$ Yeah, it looks pretty straightforward to me. If you really want me to nitpick, I would change the name of the function because I would expect
print
to print to the console, and the names of the arrays would make more sense to me if they were just called something likenumbersText
andnumbersByTensText
(but that's just personal preference). \$\endgroup\$seand– seand2011年12月13日 00:04:07 +00:00Commented Dec 13, 2011 at 0:04