CtCI 16.8: Integer to English phrase in C++

Question

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!

Answer 1

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.