7
\$\begingroup\$

I'm a C++ beginner working through Cracking the Coding Interview. This is question 16.8:

Given any integer, print an English phrase that describes the integer (e.g., "One Thou­sand, Two Hundred Thirty Four").

I have written a simple program, below, that I have confirmed works correctly to answer the question (note that I am ignoring the comma after "Thousand" in the example).

#include <iostream>
#include <vector>

// English Int: Given any integer, print an English phrase that describes the integer (e.g., "One Thou­sand, Two Hundred Thirty Four").

std::vector<int>         const magnitudes = {1000000000, 1000000, 1000, 1};
std::vector<std::string> const magnitude_names = {"Billion", "Million", "Thousand", ""};
std::vector<std::string> const number_names = {"Zero", "One", "Two", "Three", "Four", "Five", "Six", "Seven", "Eight", "Nine"};
std::vector<std::string> const tens_group_names = {"", "", "Twenty", "Thirty", "Forty", "Fifty", "Sixty", "Seventy", "Eighty", "Ninety"};
std::vector<std::string> const teens_names = {"Ten", "Eleven", "Twelve", "Thirteen", "Fourteen", "Fifteen", "Sixteen", "Seventeen", "Eighteen", "Nineteen"};


std::string name_for_group_of_3(int group);
std::string join_vector(std::vector<std::string> vector, std::string joiner);

// -1 -> "Negative One"
// 0 -> "Zero"
int main() {
    int input;
    while (1) {
        std::cout << "Input: ";
        std::cin >> input;

        if (input == 0) {
            std::cout << "Zero" << "\n";
            continue;
        }

        std::vector<std::string> result;
        if (input < 0) {
            input *= -1;
            result.push_back("Negative");
        }

        for (int i = 0; i < magnitudes.size(); i++) {
            int magnitude = magnitudes[i];
            if (input / magnitude > 0) {
                result.push_back(name_for_group_of_3(input / magnitude));
                result.push_back(magnitude_names[i]);
            }
            input %= magnitude;
        }

        std::cout << join_vector(result, " ") << "\n";
    }
}

// 0 -> ""
// 1 -> "One"
// 10 -> "Ten"
// 15 -> "Fifteen"
// 34 -> "Thirty Four"
// 456 -> "Four Hundred Fifty Six"
std::string name_for_group_of_3(int group) {
    std::vector<std::string> result;

    // group should be 0...999
    if (group < 0 || group > 999) {
        throw "Bad grouping provided!";
    }

    // Handle hundreds
    if (group / 100 > 0) {
        result.push_back(number_names[group / 100] + " Hundred");
    }

    int double_digits = group % 100;

    // Handle special case for 11...19
    if (double_digits >= 10 && double_digits < 20) {
        result.push_back(teens_names[double_digits % 10]);
    }
    else {
        // Handle tens group
        if (double_digits / 10 > 0) {
            result.push_back(tens_group_names[double_digits / 10]);
        }

        // Handle ones
        if (double_digits % 10 > 0) {
            result.push_back(number_names[double_digits % 10]);
        }
    }

    return join_vector(result, " ");
}

std::string join_vector(std::vector<std::string> vector, std::string joiner) {
    std::string str_result;
    for (int i = 0; i < vector.size(); i++) {
        str_result += vector[i];
        if (i < vector.size()-1) {
            str_result += joiner;
        }
    }

    return str_result;
}

Notes & specific questions:

  • Compiling with g++ --std=c++11 main.cpp
  • I can run this on the first 10000 integers 0.15s on my '15 MacBook Pro, roughly 15µs per run. Is this reasonable performance?
  • I opted to build vectors of strings, then join them together later, mostly to simplify dealing with the details of space placement. This feels cleaner to me, do you agree? Am I losing significant perf by using vectors vs. string concat?
  • Is it evil to be modifying input as I do during main, with input %= magnitude;? It feels a bit odd to modify the original user input.
  • Is vector a terrible argument name in join_vector?

I, of course, don't really know what else to ask - appreciate any & all pointers!

\$\endgroup\$
4
\$\begingroup\$

General

Compile with -Wall -Wextra -pedantic-errors. Warnings are preferable to runtime problems. Sometimes I also compile with -ftrapv to avoid signed overflow.

You are doing much of the work inside main. This violates the one responsibility principle. Consider extracting a function to_English to do the actual work. Also, your program becomes clueless after reaching EOF or invalid input. A better main function looks like:

int main()
{
    for (int num; std::cout << "Input: ", std::cin >> num;)
        std::cout << to_English(num) << "\n";
}

In this case, the input process is simple, so I put it in the loop condition. You may want to extract as a separate get_number function for more sophisticated input.

You do not need to store the strings in vectors. Just concatenate them in place.

Code

#include <iostream>
#include <vector>

You are missing #include <string>.

std::vector<int>         const magnitudes = {1000000000, 1000000, 1000, 1};

Using int to hold values outside of the range \$-32\,768 \le n < 32\,768\$ is nonportable. Use, for instance, int_least32_t, instead. (You need to #include <cstdint>) Write an alias for this to express the intent more explicitly: using number_t = std::int_least32_t, and use it consistently throughout the code.

std::string name_for_group_of_3(int group);

The name of the function is not entirely clear, but I cannot think of a better name either. Maybe add a comment.

std::string join_vector(std::vector<std::string> vector, std::string joiner);

Don't pass vectors and strings by value. Pass by const reference instead. Also, join_strings may be a better name in my opinion.

for (int i = 0; i < vector.size(); i++) {
for (int i = 0; i < vector.size(); i++) {
if (i < vector.size()-1) {

These lines trigger -Wsign-compare. Use std::size_t or std::vector<std::string>::size_type instead of int. Use ++i instead of i++.

// group should be 0...999
if (group < 0 || group > 999) {
    throw "Bad grouping provided!";
}

Never throw a string literal. Throw a std::invalid_argument instead. Also, an assertion may be better for logical errors.

\$\endgroup\$

Your Answer

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

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