patterncppMinor
Integer to English Conversion
Viewed 0 times
conversionenglishinteger
Problem
I wrote some code to translate numbers ( for now just positive, up to the 32bit limit ) into English words.
Everything works and I'm happy. I searched through the site looking for a comparison code but I couldn't find a C++ version that goes above 999 to use as example
I had a few assumptions:
I never wrote much code ( yes this is a big program for me ), so I have no idea how to manage it correctly, I think it's pretty much spaghetti-code at the moment, that's why I tried adding some documentation to clarify things.
The algorithm I used feels like it's overcomplicated and more patched together than planned out.
```
#include // std::cout, std::cin, std::endl
#include // std::vector
#include // std::reverse
const std::vector first_twenty_vocabular = {
"zero ",
"one ",
"two ",
"three ",
"four ",
"five ",
"six ",
"seven ",
"eight ",
"nine ",
"ten ",
"eleven ",
"twelve ",
"thirteen ",
"fourteen ",
"fifteen ",
"sixteen ",
"seventeen ",
"eighteen ",
"nineteen " };
const std::vector magnitude_vocabular = {
"hundred ",
"thousand ",
"million ",
"billion "};
const std::vector decine_vocabular = {
"twenty ",
"thirty ",
"fourty ",
"fifty ",
"sixty ",
"seventy ",
"eighty ",
"ninety "};
// HERE THE MAGIC HAPPENS.
std::string Stringer(std::vector src);
// Translate the number up to the hundreds, if the number is bigger it sends it to MagnitudeSplitte() for splitting it up.
std::string MagnitudeSplitter(std::vector src);
// Splits a number too big for Stringer to handle into smaller chunks (using VectorSplitter()) of max 3 digits, translate them with Stringer() and appends the right magnitude.
std::vector VectorSplitter(std::vector
Everything works and I'm happy. I searched through the site looking for a comparison code but I couldn't find a C++ version that goes above 999 to use as example
I had a few assumptions:
- No need for and, just spaces ( Ex: two hundred three )
I never wrote much code ( yes this is a big program for me ), so I have no idea how to manage it correctly, I think it's pretty much spaghetti-code at the moment, that's why I tried adding some documentation to clarify things.
The algorithm I used feels like it's overcomplicated and more patched together than planned out.
```
#include // std::cout, std::cin, std::endl
#include // std::vector
#include // std::reverse
const std::vector first_twenty_vocabular = {
"zero ",
"one ",
"two ",
"three ",
"four ",
"five ",
"six ",
"seven ",
"eight ",
"nine ",
"ten ",
"eleven ",
"twelve ",
"thirteen ",
"fourteen ",
"fifteen ",
"sixteen ",
"seventeen ",
"eighteen ",
"nineteen " };
const std::vector magnitude_vocabular = {
"hundred ",
"thousand ",
"million ",
"billion "};
const std::vector decine_vocabular = {
"twenty ",
"thirty ",
"fourty ",
"fifty ",
"sixty ",
"seventy ",
"eighty ",
"ninety "};
// HERE THE MAGIC HAPPENS.
std::string Stringer(std::vector src);
// Translate the number up to the hundreds, if the number is bigger it sends it to MagnitudeSplitte() for splitting it up.
std::string MagnitudeSplitter(std::vector src);
// Splits a number too big for Stringer to handle into smaller chunks (using VectorSplitter()) of max 3 digits, translate them with Stringer() and appends the right magnitude.
std::vector VectorSplitter(std::vector
Solution
From the top down, first, given what you want to do, your function had better look like this:
Your
That'll just overwrite your
The rest of
Which would be exactly equivalent to:
On top of that, you're copying your objects at every point instead of taking by reference to const. I find it very hard to follow your
Let's start over. With English number words, the way to do it is to divide the number into blocks of 3 and do each one. So I would expect a helper function like;
Which we could use like:
std::string toEnglish(int x);Your
Stringer() function sort of does this, but with a really weird name. In fact, all your functions have really weird names. Splitting into a vector of digits doesn't seem particularly useful to the problem at hand either. And your negative check is definitely wrong:for (int i = 0; i < len; ++i) { src[i] = -i; }That'll just overwrite your
src with values that had nothing to do with it.The rest of
Stringer()'s logic is really confusing as well. So we have:if (len == 1) {
std::string add = units(src);
num_string.append(add);
} else if (len == 2) {
std::string add = decine(src);
num_string.append(add);
} else if (len == 3) {
std::string add = hundreds(src);
num_string.append(add);
} else {
return MagnitudeSplitter(src);
}
return num_string;Which would be exactly equivalent to:
switch (len) {
case 1: return units(src);
case 2: return decine(src);
case 3: return hundreds(src);
default: return MagnitudeSplitter(src);
}MagnitudeSplitter then checks for the length being 3, which we know will never happen. But really, why are these different cases at all? On top of that, you're copying your objects at every point instead of taking by reference to const. I find it very hard to follow your
decine and hunrdeds functions. I am not sure if they're correct. I think the best code review I could give you is...Let's start over. With English number words, the way to do it is to divide the number into blocks of 3 and do each one. So I would expect a helper function like;
/*
Given a number 1-999, convert it to English.
Examples:
2 -> "two"
127 -> "one hundred twenty seven"
*/
std::string blockToEnglish(int x);Which we could use like:
std::string toEnglish(int x) {
if (x == 0) return "zero";
std::vector blocks;
while (x > 0) {
blocks.push_back(x % 1000);
x /= 1000;
}
std::vector block_words;
for (size_t i = 0; i != blocks.end(); ++i) {
if (blocks[i]) {
block_words.append(blockToEnglish(blocks[i]));
}
}
// TODO as exercise: combine block_words into one
// string with the "millions", etc separators
// as well as handle negatives
}Code Snippets
std::string toEnglish(int x);for (int i = 0; i < len; ++i) { src[i] = -i; }if (len == 1) {
std::string add = units(src);
num_string.append(add);
} else if (len == 2) {
std::string add = decine(src);
num_string.append(add);
} else if (len == 3) {
std::string add = hundreds(src);
num_string.append(add);
} else {
return MagnitudeSplitter(src);
}
return num_string;switch (len) {
case 1: return units(src);
case 2: return decine(src);
case 3: return hundreds(src);
default: return MagnitudeSplitter(src);
}/*
Given a number 1-999, convert it to English.
Examples:
2 -> "two"
127 -> "one hundred twenty seven"
*/
std::string blockToEnglish(int x);Context
StackExchange Code Review Q#97034, answer score: 5
Revisions (0)
No revisions yet.