Why aren't these strings equal

Question

I'm learning C++ out of curiosity - my current task is to check if a string is a palindrome. What should be happening is the string is reversed, and I use the Equal To (==) operator to compare the original and reversed string. But for some reason, they're all returning as false, and I can't understand why.

#include <iostream>
using namespace std;

string reverse_string(string text) {
  int string_size = text.size();
  string reversed = "";
  while (string_size >= 0) {
    reversed += text[string_size];
    string_size--;
  };
  return reversed;
}

bool is_palindrome(string text) {
  if (reverse_string(text) == text) {
    return true;
  };
  return false;
}

int main() {
  cout << is_palindrome("madam") << "\n";
  cout << is_palindrome("ada") << "\n";
  cout << is_palindrome("lovelace") << "\n";
}

For the record, I'm doing this via Codeacademy - It's returning false (0) for both the browser CMD prompt, as well as local.

Something I'm noticing that if I try to log the comparison (reversed_string == text), I'm getting a single random integer. I'm not sure why that's happening; shouldn't it just be returning 0 or 1?

Answer 1

The other answers explain how you've failed to reverse your string; but you don't need to copy it: std::string has a reverse view built in:

    bool is_palindrome(const std::string & text) {
      return std::equal(text.begin(), text.end(), text.rbegin());
    }

Or, if you want a string that is the reversed, you can construct it from the pair of iterators:

    std::string reversed(const std::string & text) {
        return { text.rbegin(), text.rend() };
    }

Answer 2

C++ indexing is zero-based. Thus string_size, while initially being the length of the string, is one past the end of the string.

So in your loop’s first iteration you are indexing past the string, leading to the zero termination character ('\0') being returned. To fix this it’s enough to decrement the index just before performing the indexing operation.

Answer 3

The index of the last character in a string is that string's size (or length) minus one (because array and other container indexes in C++ are zero-based). So, in your reverse_string function you need to decrement the index before copying it. You can do this conveniently with the pre-decrement --x operator, as shown below:

string reverse_string(string text)
{
    size_t string_size = text.size(); // Best to use size_t type.
    string reversed = "";
    while (string_size > 0) {
        reversed += text[--string_size]; // Decrement index BEFORE copying
    } // No need for a semicolon here!
    return reversed;
}

Note also the other minor changes I have made to your code.