6
\$\begingroup\$

I wrote this converter code. It can convert a number from 1-100 into words. It works perfectly, but it seems to be too complicated to me.

Can you tell me whether it's okay or not?

#include <iostream>

int main()
{
    int number;
    int first_digit;
    int second_digit;
    std::cout << "Enter the number: ";
    std::cin >> number;
    first_digit = number/10;
    second_digit = number%10;
    if(number >= 11 && number <= 20)
    {
        switch(number)
        {
            case 11:
                std::cout<<"eleven";
                break;

            case 12:
                std::cout<<"twelve";
                break;
            case 13:
                std::cout<<"thirteen";
                break;
            case 14:
                std::cout<<"fourteen";
                break;
            case 15:
                std::cout<<"fifteen";
                break;
            case 16:
                std::cout<<"sixteen";
                break;
            case 17:
                std::cout<<"seventeen";
                break;
            case 18:
                std::cout<<"eighteen";
                break;
            case 19:
                std::cout<<"nineteen";
                break;
        }
    }
    else
    {
        switch(first_digit)
        {
            case 1:
                if(second_digit == 0)
                    std::cout<<"ten";
                break;
            case 2:
                std::cout<<"twenty";
                break;
            case 3:
                std::cout<< "thirty";
                break;
            case 4:
                std::cout<<"fourty";
                break;
            case 5:
                std::cout<<"fifty";
                break;
            case 6:
                std::cout<<"sixty";
                break;
            case 7:
                std::cout<<"seventy";
                break;
            case 8: 
                std::cout <<"eighty";
                break;
            case 9:
                std::cout <<"ninety";
                break;
            case 10:
                std::cout <<"one hundred";
                break;
        }
        if(first_digit > 1 && number != 100) std::cout<<"-";
        switch(second_digit)
        {
            case 1:
                std::cout<<"one";
                break;
            case 2:
                std::cout<<"two";
                break;
            case 3:
                std::cout<<"three";
                break;
            case 4:
                std::cout<<"four";
                break;
            case 5:
                std::cout<<"five";
                break;
            case 6:
                std::cout<<"six";
                break;
            case 7:
                std::cout<<"seven";
                break;
            case 8:
                std::cout<<"eight";
                break;
            case 9:
                std::cout<<"nine";
                break;
            default:
                break;

        }
    }
    return 0;
}
\$\endgroup\$
5
\$\begingroup\$

I've never tackled such a problem before, so I'll proceed with this cautiously.

  • You don't need to declare those three variables and then assign to them. Instead, initialize them right away where they're first used.

    You can also move number right above the cin for slightly closer scope.

    std::cout << "Enter the number: ";
    int number;
    std::cin >> number;
    int first_digit = number/10;
    int second_digit = number%10;
    
  • I suppose switch is an okay choice for this program. Either way, the switch can still be condensed while making the program more modular (with functions).

    Here's what 11-20 could look like within its own function:

    std::string elevenThroughTwenty(unsigned int number)
    {
        switch (number)
        {
            case 11: return "eleven";
            // ...
            case 20: return "twenty"; // you forgot the 20 in your code
    
            // throw an exception if number not in the switch
            // include <stdexcept> to use this
            default: throw std::logic_error("Not 11 through 20");
        }
    }
    

    Separating them as such helps with readability, organization, and conciseness. You will also need proper defaults so that invalid numbers don't cause crippling errors.

\$\endgroup\$
  • \$\begingroup\$ Thank you :) but I have a question: what does O(1) mean? \$\endgroup\$ – mitya221 Aug 30 '13 at 20:19
  • \$\begingroup\$ @mitya221: Constant time. Meaning, one pass no matter what. Example: arr[7] = 25. \$\endgroup\$ – Jamal Aug 30 '13 at 20:32
  • 1
    \$\begingroup\$ @Jamal I don't think switches are "constant time" in the way that you mean. A switch with N different cases will (I'm pretty sure) generate N different branch statements that the program would proceed through sequentially. \$\endgroup\$ – Alec Aug 30 '13 at 20:38
  • \$\begingroup\$ @alecbenzer: Noted. The OP didn't know what O(1) meant anyway, so I've explained that. \$\endgroup\$ – Jamal Aug 30 '13 at 20:46
  • 1
    \$\begingroup\$ @mitya221: As already brought to my attention, O(1) is not guaranteed here (but my comment still stands). Also, most of the examples I've found on this use arrays. I suppose it doesn't make a huge difference (although still different times), but it's up to you. As for switches in general, I'd at least make them as condensed (but readable) as possible. They can take up much physical space. \$\endgroup\$ – Jamal Aug 30 '13 at 21:15
3
\$\begingroup\$

I would probably make some arrays and do lookups in them instead of using switches: http://ideone.com/44IB0J

#include <iostream>

using std::cout;
using std::string;
using std::endl;

string toWords(int num) {
  if (num > 100 || num < 1) {
    throw "unsupported";
  }
  if (num == 100) {
    return "one hundred";
  }

  const string kSpecialCases[] = {"ten", "eleven", "twelve", "thirteen", "fourteen", "fifteen", "sixteen", "seventeen", "eighteen", "nineteen"};
  if (10 <= num && num <= 19) {
    return kSpecialCases[num - 10];
  }

  const string kOnesPlaces[] = {"one", "two", "three", "four", "five", "six", "seven", "eight", "nine"};
  const string kTensPlaces[] = {"twenty", "thirty", "forty", "fifty", "sixty", "seventy", "eighty", "ninety"};
  if (num < 10) {
    return kOnesPlaces[num - 1];
  } else if (num % 10 == 0) {
    return kTensPlaces[num / 10 - 2];
  } else {
    return kTensPlaces[num / 10 - 2] + " " + kOnesPlaces[num % 10 - 1];
  }
}

int main() {
  cout << toWords(1) << endl;
  cout << toWords(100) << endl;
  cout << toWords(12) << endl;
  cout << toWords(29) << endl;
  cout << toWords(46) << endl;
  return 0;
}
\$\endgroup\$
2
\$\begingroup\$

Code will break on 20

if(number >= 11 && number <= 20)
{
    switch(number)
    {
   ......
        case 19:
            std::cout<<"nineteen";
            break;
       /// No Twenty here.
    }
}
\$\endgroup\$
1
\$\begingroup\$

Here's my take on the problem making use of recursion. This works for any number between 0 (zero) and 999,999,999 (nine hundred ninety nine million nine hundred ninety nine thousand nine hundred ninety nine).

Note: Due to making use of initializer lists, this will only work in C++11.

#include <vector>
#include <iostream>
#include <stdexcept>

const std::vector<std::string> first14 = {"zero", "one", "two", "three", "four", "five", "six", "seven", "eight", "nine", "ten", "eleven", "twelve", "thirteen", "fourteen" };
const std::vector<std::string> prefixes = {"twen", "thir", "for", "fif", "six", "seven", "eigh", "nine"};

std::string inttostr( const unsigned int number )
{
    if( number <= 14 )
        return first14.at(number);
    if( number < 20 )
        return prefixes.at(number-12) + "teen";
    if( number < 100 ) {
        unsigned int remainder = number - (static_cast<int>(number/10)*10);
        return prefixes.at(number/10-2) + (0 != remainder ? "ty " + inttostr(remainder) : "ty");
    }
    if( number < 1000 ) {
        unsigned int remainder = number - (static_cast<int>(number/100)*100);
        return first14.at(number/100) + (0 != remainder ? " hundred " + inttostr(remainder) : " hundred");
    }
    if( number < 1000000 ) {
        unsigned int thousands = static_cast<int>(number/1000);
        unsigned int remainder = number - (thousands*1000);
        return inttostr(thousands) + (0 != remainder ? " thousand " + inttostr(remainder) : " thousand");
    }
    if( number < 1000000000 ) {
        unsigned int millions = static_cast<int>(number/1000000);
        unsigned int remainder = number - (millions*1000000);
        return inttostr(millions) + (0 != remainder ? " million " + inttostr(remainder) : " million");
    }
    throw std::out_of_range("inttostr() value too large");
}

int main()
{
    try {
        for( int i = 0; i <= 999999999; i++ )
            std::cout << i << " = " << inttostr(i) << std::endl;
    } catch( std::exception& ex ) {
        std::cerr << "Error: " << ex.what() << std::endl;
    }
    return 0;
}
\$\endgroup\$
0
\$\begingroup\$

This is how I would do it:

#include <iostream>
#include <vector>
#include <map>

std::vector<std::string> const names[]= {{"", "one", "two", "three", "four", "five", "six", "seven", "eight", "nine",
                                         "ten", "eleven", "twelve", "thirteen", "fourteen", "fifteen", "sixteen", "seventeen", "eighteen", "nineteen" },
                                         {"-", "-", "twenty--", "thirty--", "forty--", "fifty--", "sixty--", "seventy--", "eighty--", "ninety--"}
                                        };

void print(int& count, int unit, int xx, std::string const& units)
{
    int                 div     = unit * (xx==0?1:10);
    int                 index   = count / div;
    int                 sub     = index * div * (index >=2 ? 1 : 0);
    std::string         name    = names[xx][index];

    if (name[name.size()-1] == '-') {
        name.erase(name.size()-1);
        index   = (count-sub)/unit;
        name.append(names[0][index]);
    }
    if (name != "") {
        std::cout << name << " " << units << " ";
    }
    count   = count - (count/unit*unit);
}
void printNumber(int number)
{
    if (number == 0) {
        std::cout << "zero";
        return;
    }
    print(number, 1000000, 0, "million");
    print(number, 100000,  0, "hundred");
    print(number, 1000,    1, "thousand");
    print(number, 100,     0, "hundred");
    print(number, 1,       1, "");
}
int main()
{
    int number;
    std::cin >> number;
    printNumber(number);
    std::cout << "\n";
}

Result:

> ./a.out
845
eight hundred forty-five
> ./a.out
999999
nine hundred ninety-nine thousand nine hundred ninety-nine
> ./a.out
1000001
one million one
\$\endgroup\$

protected by Community Dec 6 '14 at 9:04

Thank you for your interest in this question. Because it has attracted low-quality or spam answers that had to be removed, posting an answer now requires 10 reputation on this site (the association bonus does not count).

Would you like to answer one of these unanswered questions instead?

Not the answer you're looking for? Browse other questions tagged or ask your own question.