7
\$\begingroup\$

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;
}
\$\endgroup\$
  • \$\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 Dec 14 '11 at 17:08
11
\$\begingroup\$

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;
}
| improve this answer | |
\$\endgroup\$
  • 1
    \$\begingroup\$ Use boost::array if std::array is not supported. \$\endgroup\$ – rightfold Jan 1 '12 at 16:17
6
\$\begingroup\$
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;
}
| improve this answer | |
\$\endgroup\$
5
\$\begingroup\$

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;
}
| improve this answer | |
\$\endgroup\$
  • \$\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 Dec 13 '11 at 8:34
2
\$\begingroup\$

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
| improve this answer | |
\$\endgroup\$
  • \$\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 Dec 12 '11 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 like numbersText and numbersByTensText (but that's just personal preference). \$\endgroup\$ – seand Dec 13 '11 at 0:04

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy