Number to English word converter

This is a simple program that I wrote. It takes a 64-bit unsigned int as input and returns its English name.

```
#include
#include
#include
#include
using namespace std;
int container[7]; /// every element contains a number between 0 and 999
string conv0to19(uint64_t); /// converts 1 to 19 to its name
string conv_dec(uint64_t); /// converts 20,30...,90 to its name
string convert(uint64_t); ///converts a number from of range uint64_t to its name
void fillContainer(uint64_t);
string convert1to999(uint64_t, int);

string name[]={"","thousand","million","billion","trillion","quadrillion","quintillion"};

string conv0to19(uint64_t a)
{
switch (a)
{
case 0:return ""; break;
case 1:return "one "; break;
case 2:return "two "; break;
case 3:return "three ";break;
case 4:return "four "; break;
case 5:return "five "; break;
case 6:return "six "; break;
case 7:return "seven ";break;
case 8:return "eight ";break;
case 9:return "nine "; break;
case 10:return "ten "; break;
case 11:return "eleven "; break;
case 12:return "twelve "; break;
case 13:return "thirteen ";break;
case 14:return "fourteen "; break;
case 15:return "fifteen "; break;
case 16:return "sixteen "; break;
case 17:return "seventeen ";break;
case 18:return "eighteen ";break;
case 19:return "nineteen ";break;
}
}

string conv_dec(uint64_t a)
{
switch(a)
{
case 0:return "";break;
case 1:return "";break;
case 2:return "twenty "; break;
case 3:return "thirty "; break;
case 4:return "forty "; break;
case 5:return "fifty "; break;
case 6:return "sixty "; break;
case 7:return "seventy "; break;
case 8:return "eighty "; break;
case 9:return "ninety ";break;

}
}

string convert1to999(uint64_t a,int i)
{
if(a!=0)

Switched

Let's start with the obvious things we can improve. A return statement terminates the current function. Therefore, all your

case XY: return AB; break;

can be rewritten as

case XY: return AB;

However, you don't have any default cases in both switch statements. What happens if you use conv0to19(100)? Well, it's not obvious, because it's undefined behaviour.

For the sake of simplicity, let's use an assert for unhandled cases. But before we do that, let's have a closer look at your switch:

case 0:return "";
    case 1:return "one ";
    case 2:return "two ";
      ...
    case 18:return "eighteen ";
    case 19:return "nineteen ";

Hm. You have a continuous range of integer values. So let's use that to our advantage:

std::string conv0to19(uint64_t a) {
    static char const * const numbers[20] = 
      {"", "one", "two", "three", "four", "five", "six", "seven", "eight", "nine", "ten",
      "eleven", "twelve", "thirteen", "fourteen", "fifteen", "sixteen", "seventeen", "eighteen", "nineteen"
      };

    assert(0 <= a && a < 20);

    return numbers[a];
}

You can do the same for conv_dec. Note that I've removed the suffix space from the string. More on that later.

While we're at it and using an array, your name should:

• have a better name (especially not name, since it's a collection of several ones)

• be const, so that you don't change it by accident:

const std::string numberNames[] = {"", "thousand", "million", "billion", "trillion", "quadrillion", "quintillion"};

By the way, did you notice that I've added whitespace to separate the arguments? Remember code is written for humans (and for machines). If you want to remove a word later, it should be easy to do so, and whitespace makes it a lot easier to mark text with Ctrl+Shift (or whatever your operating system/editor uses).

Back to the beginning

int container[7];                       /// every element contains a number between 0 and 999

That is a code smell. It's a global variable, which isn't constant. And the description is, well, no really helpful. Just remove it.

Also, the 7 seems like a magic number. Why is it 7? Why not 10?

Either way, you should include cstdint instead of stdint-gcc.h, or — if your compiler is too old — stdint.h. Also, I recommend you to typedef your used number type, e.g.:

typedef uint64_t number_type;

That way you can replace the type easily, if you want to support even larger numbers later.

And using namespace std… well, it's considered bad practice for large programs, especially in headers, but you're writing only a single file here. It's up to you, but I wouldn't use it.

Using logic to reduce if-complexity

Your convert1to999 is too complex. If we didn't enter the following block:

if (a>0&&a<20)
        return conv0to19(a)+name[i];

we're guaranteed that a is larger then 20. Why? Because a isn't 0. Since it is a non-negative number, it must be then greater than zero, so a>0 holds, so a < 20 must not hold. We don't need to repeat that in the next condition:

if(a>20&&a<100&&a%10!=0)
       ^^^^

And again, your code is neigh unreadable, since you don't use any whitespace. What is easier to read:

a>=20&&a&&a<100&&a%10==0

or

a >= 20 && a && a < 100 && a % 10 == 0

I guess you now see the superfluous a in the condition, which will always be true at that point.

Next, this function should do one job. It should get a number from 0-999 and return its name, not add the suffix. So let's get rid of that and fix the if complexity. And while we're at it, let's add another function for 0-99:

std::string conv0to99(number_type a)
{
    assert(0 <= a && a < 100);

    if(a == 0) {
        return "";
    } else if (a <= 20) {
        return conv0to19(a);
    } else {
        return conv_dec(a / 10) + "-" + conv0to19(a % 10);
    }
}

std::string convert1to999(number_type a)
{
    assert(0 <= a && a < 1000);

    if(a == 0) {
        return "";
    } else if (a <= 100) {
        return conv0to99(a);
    } else {
        return conv0to19(a/100) + " hundred " + conv0to99(a % 100);
    }
}

Both functions are rather simple and easy to check. The hurdles of the <=20 are now (almost) entirely hidden in conv0to99, which is still easy enough to understand quickly.

As I said, get rid of fillContainer. You use it only in convert, so let's only have it there. Also, handle the easy case first:

``
std::string convert(uint64_t a)
{
if(a == 0) {
return "zero";
}

std::string output;
int container[7];

for(int i = 0; i =0; i--) {
if(container[i] != 0) {
// we add the name now here instead of in convert1to999
output += convert1to999(container[i]) + " " + name[i] + " ";
}
}

// if your compiler doesn't have output.back()`, use your old check
while(output.back() == ' ') {
output.p

StackExchange Code Review Q#152523, answer score: 14