Skip to main content
edited body
Source Link
Davislor
  • 8.4k
  • 17
  • 37
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: whileif (left++*left++ <!= right*right--)
      --right;
    } // end while
  } // end if
  return true;  
}
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;  
}
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: if (*left++ != *right--)
      --right;
    } // end while
  } // end if
  return true;  
}
deleted 3 characters in body
Source Link
Davislor
  • 8.4k
  • 17
  • 37

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 behaviora dangerous bug!

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 sureparanoid in case someone mucks up the loop.. 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.

#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 imputinput \"" << s << "\"." << endl;

  return EXIT_SUCCESS;  
}

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!

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.

#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;  
}

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] a dangerous bug!

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 paranoid in case someone mucks up the loop.. 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.

#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 input \"" << s << "\"." << endl;

  return EXIT_SUCCESS;  
}
Source Link
Davislor
  • 8.4k
  • 17
  • 37

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;  
}