String text to a Number Converter

[NathanOliver]

Hey All,

I just finished my code and it seams to work fine. I'm looking for advice on improving it or making it easier to understand. Its not Fully complete because I only have to set up to take in a number less than 2147483648 because i am using an int. I know that can be changed, but before I get it fully polished I would appreciate your insights.

Thanks for Reading
Nathan

#include <iostream>
#include <string>
#include <vector>
#include <algorithm>

int GetNumber(std::string number)
{
	std::string words[31] = {"one", "two", "three", "four", "five", "six", "seven",
 "eight", "nine", "ten", "eleven", "twelve", "thirteen", "fourteen", "fifteen", "sixteen", "seventeen", "eightteen","nineteen", "twenty", "thirty", "fourty", "fifty", "sixty", "seventy", "eighty", "ninety","hundred", "thousand", "million", "billion"};
	int numbers[31] = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20,30, 40, 50, 60, 70, 80, 90, 100, 1000, 1000000, 1000000000};
	for (int i = 0; i < 31; i++)
	{
		if (number == words[i])
			return numbers[i];
	}
	return 0;
}

int ConvertTextToNumber(std::string sentence)
{
	int number = 0;
	std::vector<std::string> parts;
	std::string tempString;
	size_t size = sentence.size();
	for (size_t i = 0; i < size; i++)
	{
		if (sentence[i] == ' ')
		{
			parts.push_back(tempString);
			tempString.clear();
		}
		else
		{
			tempString += sentence[i];
		}
	}
	parts.push_back(tempString);
	size = parts.size();
	std::vector<std::string> formula;
	formula.push_back(parts[0]);
	for (size_t i = 1; i < size; i++)
	{
		if (parts[i] == "and")
			continue;
		if (parts[i] == "hundred" || parts[i] == "thousand" || parts[i] == "million" || parts[i] == "billion")
		{
			formula.push_back("*");
		}
		else
		{
			formula.push_back("+");
		}
		formula.push_back(parts[i]);
	}
	size = formula.size();
	std::vector<int> numbers;
	int tempNumber = GetNumber(formula[0]);
	for (size_t i = 1; i < size; i++)
	{
		tempString = formula[i];
		if (tempString == "*")
		{
			tempNumber *= GetNumber(formula[++i]);
		}
		if (tempString == "+")
		{
			tempNumber += GetNumber(formula[++i]);
		}
		if (formula[i] == "thousand" || formula[i] == "million" || formula[i] == "billion")
		{
			numbers.push_back(tempNumber);
			tempNumber = 0;
		}
	}
	numbers.push_back(tempNumber);
	size = numbers.size();
	for (size_t i = 0 ; i < size; i++)
	{
		number += numbers[i];
	}
	return number;
}

int main()
{
	std::cout << ConvertTextToNumber("nine hundred eighty seven million six hundred fifty four thousand three hundred and twenty one");
	std::cin.get();
	return 0;
}

[mrnutty]

To polish it, you can combine the words[] and numbers[] into one std::pair<string,int>;
And your search in GetNumber could be just, numbers[ words.find_first_of(number) ] , but be careful about error checking. And make the parameters const-reference.

This code

std::string tempString;
	size_t size = sentence.size();
	for (size_t i = 0; i < size; i++)
	{
		if (sentence[i] == ' ')
		{
			parts.push_back(tempString);
			tempString.clear();
		}
		else
		{
			tempString += sentence[i];
		}
	}

should be a function by itself using stringstream:

std::vector<string> splitBySpace(const std::string& src){
 std::vector<string> result;
 stringstream stream;
 string word;
 stream << src;
 while( src >> word ){ result.push_back(word ); }
 return result;
}

Similarly there are more parts to ConvertTextToNumber that you can split into more functions.

[vijayan121]

First, a couple of suggestions regarding your coding style. I'm taking int GetNumber(std::string number) as an exmple. The comments are inline, and some of them would apply to the rest of your code as well.

// for user defined types, prefer passing by reference-to-const over passing by value
int GetNumber( const std::string& number )
{
    // static - needs to be initialized only once
    // const - just being const-correct
    // avoiding magic numbers eg. words[31], for( int i=0 ; i<31; ++i ) etc.
    static const std::string words[] = {"one", "two", "three", "four", "five", 
        "six", "seven", "eight", "nine", "ten", "eleven", "twelve", "thirteen", 
        "fourteen", "fifteen", "sixteen", "seventeen", "eighteen","nineteen", 
        "twenty", "thirty", "fourty", "fifty", "sixty", "seventy", "eighty", 
        "ninety","hundred", "thousand", "million", "billion" } ;

    // an extra zero added at the end of numbers[] 
    // to avoid special casing when 'number' is not in 'words[]'.
    static const int numbers[] = { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 
        15, 16, 17, 18, 19, 20,30, 40, 50, 60, 70, 80, 90, 100,
        1000, 1000000, 1000000000, 0 } ;

    enum { N = sizeof(words)/sizeof(*words), M = sizeof(numbers)/sizeof(*numbers) - 1 } ;

    // just making sure that we have not inadvertently omitted something
    // a compile-time assertion that the number of elements are in sync.
    typedef char N_must_be_equal_to_M_here[ N == M ] ; // C++98
    //static_assert( N==M, "N_must_be_equal_to_M_here") ; // C++1X

    // why write a loop (where it's easy to make a careless mistake or a typo)
    // when there is a std algorithm that can do the job
    return numbers[ std::find( words, words+N, number ) - words ] ;
}

In the function int ConvertTextToNumber(std::string sentence) , IMHO, you are trying to do too much.
a. parse the string to get a sequence (parts) of ws-delimited tokens
b. convert these tokens into a formula by removing the noise word "and" and inserting operators "+" or "*" as appropriate.
c. convert the tokens between the operators into numbers (we already have a function for doing this).
d. evaluate the formula to get the numeric result.

If you put each one of these steps into separate, smaller functions, your code would be easier to write, understand, test, modify etc.

std::vector< std::string > GetParts( const std::string& sentence )
{
    std::vector< std::string > parts ;

    // using an istrinstream to seperate the ws-delimited parts is much easier
    std::istringstream stm(sentence) ;
    std::string temp ;
    while( stm >> temp ) parts.push_back(temp) ;

    return parts ;
}

// again, avoiding magic constants
const std::string PLUS = "+", MULTIPLIES = "*", NOISE = "and" ;

std::vector< std::string > MakeFormula( const std::vector< std::string >& parts )
{
    static const std::string multipliers[] = { "hundred", "thousand", "million", "billion" } ;
    enum { N = sizeof(multipliers) / sizeof(*multipliers) } ;

    std::vector< std::string > formula ;

    if( parts.empty() ) return formula ; // safety check added
    else formula.push_back( parts.front() ) ;

    for( std::vector< std::string >::size_type i = 1 ; i < parts.size() ; ++i )
    {
        if( parts[i] != NOISE )
        {
            if( std::find( multipliers, multipliers+N, parts[i] ) != (multipliers+N) )
                formula.push_back(MULTIPLIES) ;
            else
                formula.push_back(PLUS) ;

            formula.push_back( parts[i] ) ;
        }
    }

    return formula ;
}

int ConvertTextToNumber( const std::string& sentence )
{
    std::vector< std::string > formula = MakeFormula( GetParts(sentence) ) ;

    // TO DO: compute the value from the formula and return it
}