Return all words which have their reverse present in a string

Problem Statement:


Given a string of words return all words which have their reverse present in the string as ( (word1 , reverseword1 ) , (word2 ,reverseword2) )

Example Case:

Input:


Sachin tendulkar is the best tseb eht nihcaS

Output:


{ ( best , tseb ) , ( the , eht) , (Sachin , nihcaS) }

My approach is to use a map and return the words if a match to the reverse of the current word is found in the map.

#include
#include
#include

using namespace std;

int main()
{
unordered_map  binMap;
string test="Sachin tendulkar is the best tseb eht nihcaS si";
int i=0,j=0;
string temp,rev;
string::iterator it;
cout<<"{";

for(i = 0, it = test.begin() ; it <= test.end(); ++it)
    {   

        if(*it==' ' || it==test.end())
        {   
            temp=test.substr(j,i-j);
            rev=string(temp.rbegin(), temp.rend());
            if(binMap[rev]==1)
            cout<<"( "<<rev<<","<<temp<<" ), ";
            else
            binMap[temp]=1;
            j=i+1;
        }
        i++;
    }
cout<<"}";
return 0;
}

Is this the most optimal way to solve the problem or am I doing something wrong?

I have observed a few things that may help you improve your code.

Use library functions

One of the things the code does is to separate words in the input. However, this could be done more easily by using a std::stringstream:

stringstream in(test);
string word;
while (in >> word) {
    // test each word
}

Use more whitespace to enhance readability of the code

Instead of crowding things together like this:

cout<<"( "<<rev<<","<<temp<<" ), ";

most people find it more easily readable if you use more space:

cout << "( " << rev << ", " << word << " ) ";

Only print words once

Right now, with this input string:

string test="Sachin tendulkar is si is the best tseb eht nihcaS si";

The word "is" gets reported twice.


{( is,si ), ( best,tseb ), ( the,eht ), ( Sachin,nihcaS ), ( is,si ), }

One way to handle that is to note when each word is actually reported and to only do it once. Here's one way to do that:

stringstream in(test);
enum state { DETECTED=1, REPORTED };
unordered_map  binMap;

string word;
cout > word) {
    string rev{word.rbegin(), word.rend()};
    if (binMap[rev] == DETECTED && binMap[word] != REPORTED) {
        cout << "( " << rev << ", " << word << " ) ";
        binMap[word] = REPORTED;
    } else {
        binMap[word] = DETECTED;
    }
}
cout << "}\n";

StackExchange Code Review Q#92751, answer score: 4