Google Code Jam - T9 Spelling with C++

I am trying to improve my coding skills. As a result, I solved the T9 Spelling question from the Google Code Jam dashboard. I will appreciate it if anyone could critique the code, and provide illustrations (if possible) of how I could make my code clean and simple. For me, simple is really beautiful.

Google judges the output of the code to be correct. Here is the problem statement.

#include 
#include 
#include 
#include 
#include 

using namespace std;
string process_ln(string line);

int main(){
    ifstream infile;
    ofstream outfile;
    infile.open("C-large-practice.in");
    outfile.open("C-large-practice-result.in");

    int i=0,n=1;
    int N;
    string line;
    vector all;

    infile>>N;  //number of test cases
    while(i val,temp;
    vector v;
    string keep;

for(j=0; j0 && val[i]==val[i-1])
     keep +=" ";
  keep += temp[i];
   }//end for loop
   return keep;
}//end process_ln

I see several opportunities for simplification and improvement in your main() function:

int main(){
    ifstream infile;
    ofstream outfile;
    infile.open("C-large-practice.in");
    outfile.open("C-large-practice-result.in");

std::ifstream and std::ofstream both have constructors that take the filename to open as an argument. You can collapse these lines to:

ifstream infile("C-large-practice.in");
    ofstream outfile("C-large-practice-result.in");

You don't need the all vector; see below for why.

int i=0,n=1;
    int N;
    string line;
    vector all;

This piece of code skips reads in the number of lines remaining in the file, then has special case handling to discard the remainder of the line:

infile>>N;  //number of test cases
    while(i<N+1){
        if(i<1){
        getline(infile,line);
        i++;
    }

Instead of putting a special case inside the while-loop, move that std::getline() call to above the while loop. Then the whole thing simplifies to:

infile>>N;  //number of test cases
    getline(infile,line); // Discard the rest of the line.

    while(i < N) {
    // Note: condition in the while() has changed because we don't have 
    // the extra line at the start
        string result;
        getline(infile,line);
        result = process_ln(line);
        all.push_back(result);
        outfile<<"Case #"<<n<<": "<<all[i-1]<<endl;
        n++,i++;
    }

You're storing the processed line in a vector, but the only place you're using that vector is right on the next line, where you output the string that you just put in there. So you could simplify further to:

string result = process_ln(line);
        outfile << "Case #" << n << ": " <<  result << endl;

This eliminates the need for the all vector, as mentioned above. Note that I've combined the declaration and initialization of result. This is generally a good habit to get into when using C++.

Another useful optimization to note is that the return value from std::getline() is the stream that is passed as its first argument, and streams have an implicit conversion to bool so they can be used as the conditional in while() or for() loops. If you use the return from getline() as the conditional part of a for() loop, you can declare the string in the same for() statement:

int n = 1;
    for(string line; getline(infile, line); ++n) {
        string result = process_ln(line);
        outfile << "Case #" << n << ": " << result <<endl;
    }

Then you don't need to declare i, line or all earlier in main()

The return at the end of main() is not necessary in C++; if you omit it, the compiler will generate code for the return 0 for you.

return 0;
}

In the process_ln() function, there are also some opportunities for improvement:

string process_ln(string line){
    int j;
    char letter;
    vector val,temp;
    vector v;
    string keep;

std::string has an indexing operator, so you don't need to extract each character from line here, you can simply index into it in the switch statement below and remove this code:

for(j=0; j<line.length(); j++)
    v.push_back(line[j]);

The C++ standard library has the std::ostringstream class which is very handy for building up strings one piece at a time, so in the big switch statement, where you've got temp.push_back("....") in your code, you could replace it with:

std::ostringstream  keep;
/// process the line, building it up a piece at a time.
return keep.str();

In the for loop, you're storing all the digits that you use in the val vector. You really only need the current and previous digits to keep track of this. You're also storing all the fragments for each digit, where you really only need to keep one. Since your original code also swallowed invalid input, I'm doing the same here:

std::ostringstream  keep;

        // Note: initialized to an invalid digit value to guarantee that we|
        // don't inject a space at the start of the output string.
        char    last_digit = 'X';
        char    this_digit;
        string  digits;

        for(int i = 0; i < line.length(); i++){

        switch(line[i]){
            case ' ': this_digit = '0'; digits = "0"; break;
            case 'a': this_digit = '2'; digits = "2"; break;
            case 'b': this_digit = '2'; digits = "22"; break;
            /// and so on for the rest of the letters.
            default : this_digit = 'X'; digits = ""; break;
        }//end switch

        if(this_digit == last_digit)
        {
            keep << " ";
        }
        keep << digits;
        last_digit = this_digit;
    }//end for loop
    return keep;
}//end process_ln

Note that if you follow all of my advice, you won't need any of the variables j, letter, val, temp or v declared at the start of the function (though I admit I did introduce a few new ones).

StackExchange Code Review Q#58848, answer score: 12