HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

Google Code Jam - reverse words

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
googlereversewordscodejam

Problem

As a sequel from my previous question, I decided to solve the next problem statement from the GCJ dash board. Any thorough critique, this time, with much attention to skill and technicalities would be much appreciated.

I have few questions:

-
According to the problem statement, the number of test cases was to be read from the first line in the file but I didn't need to use it. Will this have any consequence that I do not yet know?

-
You will see from the code that, I read a line without using it (only for formatting purposes). Is there a better way to handle such a situation?

  • The for-loop in the rvrs_ln function throws segmentation fault if I do not reduce the number of elements by 1.



Here is the link to problem statement.

#include 
#include 
#include 
#include 
#include 

std::string rvrs_ln(const std::string& line);

int main(){
   std::ifstream infile("B-small-practice.in");
   std::ofstream oufile("B-small-practice-result.out");
   if(!infile)
    std::cout>test_cases;      //this is never used
   getline(infile,line);    //discards first line

   for(; std::getline(infile,line); x++){
       std::string all = rvrs_ln(line);
       oufile hold;
    int count_wrd = 0;

    while(buffer>>wrd){
        hold.push_back(wrd);
        count_wrd++;
    }
    for(int i=count_wrd-1; i>=0; --i){
        rvrs += hold[i];
        rvrs += " ";
    }
    return rvrs;
}

Solution

-
You report errors if any of the files fails to open, but continue execution anyway. If either of them fails to open, you should also return EXIT_FAILURE. In addition, you should be writing the errors to cerr instead of cout.

On those same output lines, you're indenting by just one space, but you indent by four spaces everywhere else. You should try to keep the spacing consistent.

-
Don't cram lines like this:

oufile<<"Case #"<<x<<": "<<all<<std::endl;


Add more whitespace between operators for readability:

oufile << "Case #" << x << ": " << all << std::endl;


Be sure to do this everywhere, not just with this line.

-
You don't need to close files yourself, unless you're needing to check for errors. Since you're not, you can just remove these function calls.

-
Since count_wrd is incremented with each call to hold.push_back(), you can just use hold.size() in place of count_wrd. You could also rename this vector to words.

-
This doesn't need to be two separate lines:

rvrs += hold[i];
rvrs += " ";


It can just be one:

rvrs += hold[i] + " ";

Code Snippets

oufile<<"Case #"<<x<<": "<<all<<std::endl;
oufile << "Case #" << x << ": " << all << std::endl;
rvrs += hold[i];
rvrs += " ";
rvrs += hold[i] + " ";

Context

StackExchange Code Review Q#58977, answer score: 5

Revisions (0)

No revisions yet.