Hello.
I'm doing an exercise whereby I need to test if a word is a palindrome.

My code for the function is as follows:

#include "test_palindrome.h"

using std::string;

bool is_it_palindrome(const string s)
{
    bool palindrome = true;

    for(string::const_iterator iter = s.begin(); iter != s.end(); ++iter)
    {
        string::const_reverse_iterator rit = s.rbegin();
        if(*iter!=*rit)
            palindrome = false;
        ++rit;
    }
    return palindrome;
}

It compiles but when I enter any word, regardless of whether it is a palindrome or not, it comes up as false.

Why is this?

Cheers.

Hi,

string::const_reverse_iterator rit = s.rbegin();

This should be outside the loop(just before the loop). You are creating a new iterator everytime you enter the loop.

PS: This might be an interesting read: Scope of variables

Edited 4 Years Ago by myk45

for(string::const_iterator iter = s.begin(); iter != s.end(); ++iter)What does this line do?

string::const_reverse_iterator rit = s.rbegin();
What does this line do?

What do the variables defined (iter, rit) point to or indicate.

Why make pointer to the characters? Why not generate subscripts and just use them?

string::const_reverse_iterator rit = s.rbegin();
This should be outside the loop(just before the loop)

Yes.

And you need not go all the way to end(); upto the middle of the string would suffice.

Which would make the code equivalent to:

bool is_palindrome( const std::string& str )
{ return std::equal( str.begin(), str.begin() + str.size()/2, str.rbegin() ) ; }

http://en.cppreference.com/w/cpp/algorithm/equal

Edited 4 Years Ago by vijayan121

Thanks myk45. I should have seen that. \sigh.

In re WaltP's post; The reason I took this approach was because the chapter of "Accelerated C++" was all about iterators and seemed to want me to go down this path.

Thanks for the help guys.

In re WaltP's post; The reason I took this approach was because the chapter of "Accelerated C++" was all about iterators and seemed to want me to go down this path.

OK. That makes sense.

This question has already been answered. Start a new discussion instead.