Your code has a bug in it when you pass in string("")
or string()
. One other thing I’d like to point out is that your comment about how it saves time to check that num_to_parse == 1
before entering the loop isn’t quite correct. And you should always be wary when you find yourself writing a second check inside your loop that runs continue
. There’s a better way.
It’s never necessary to check the middle letter of an odd-length string, because it’s identical to itself. So you can check i < num_to_parse/2
and, when num_to_parse
is 1, the program will immediately fall through the loop.
You do, however, need to check that the string size is not zero, or else you will initialize point_end
to -1. That makes inputString[point_end]
undefined behavior!
Here’s a fixed version.
bool is_palindrome2(const std::string& s)
{
if (s.size() > 0) {
const size_t last = s.size()-1;
const size_t sentinel = s.size()/2;
for (size_t i = 0; i < sentinel; ++i )
if (s[i] != s[last-i])
return false;
} // end if
return true;
}
Many programmers don’t like using unsigned indices that way; it might be safer to add an assertion that last >= i
, just to be absolutely sure. In particular, Google would tell you to use signed types such as ptrdiff_t
for loop indices and Microsoft to use rsize_t
, so that you can catch underflow bugs more easily.
Since one of the commenters asked, why not use iterators, here’s a version that uses iterators. On an architecture without machine-language indirect addressing, this might count as closer to the metal. It’s also almost identical to the optimized code a C programmer would give you, and compiles to something just as fast:
bool is_palindrome1(const std::string& s)
{
if (s.size() > 0) {
std::string::const_iterator left = s.begin();
std::string::const_iterator right = s.end()-1;
while (left < right) {
if (*left != *right)
return false;
++left; // Often written: while (left++ < right--)
--right;
} // end while
} // end if
return true;
}
And a test driver:
#include <cstdlib>
#include <iostream>
#include <string>
#include <vector>
using std::cout;
using std::endl;
using std::size_t;
int main()
{
const std::vector<std::string> testcases =
{ "", "a", "aa", "ba", "abccba", "abcdcba", "abdccba" };
for ( const std::string& s : testcases )
if ( is_palindrome2(s) && is_palindrome1(s) )
cout << '\"' << s << "\" is a palindrome." << endl;
else if ( !is_palindrome2(s) && !is_palindrome1(s) )
cout << '\"' << s << "\" is not a palindrome." << endl;
else
cout << "Bug on imput \"" << s << "\"." << endl;
return EXIT_SUCCESS;
}