CODEWITHSUNDEEP
c++
benjamyan
benjamyan

Reputation: 210

Why aren't these strings equal

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?

Upvotes: 5

Views: 346

Answers (7)

Tanmay Sharma
Tanmay Sharma

Reputation: 7

    string reverse_string(string text) {
      int string_size = text.size() - 1; //changes to be made
      string reversed = "";
      while (string_size >= 0) {
        reversed += text[string_size];
        string_size--;
      }
     return reversed;
   }

indexing in C/C++ starts from (0) to (length - 1). so, if we use just text.size() it will count whole length, but we need index here. which will be text.size() - 1. Now, it is working as expected. You can try some more examples inside it like racecar, rotator, refer, etc.

Upvotes: 1

Caleth
Caleth

Reputation: 62666

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

Upvotes: 11

AmirH.Najafizadeh
AmirH.Najafizadeh

Reputation: 23

You see, when you are using strings in c++, it will create an array of chars to store that string in the memory. It puts a special character in the last house of that array as a flag that shows the end of that string. Your problem is in the 'reverse_string' function. You are adding that flag to your reverse string. Thus those two strings will not be equal.

Just need to set your while loop on step forward.

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

Upvotes: 1

The Acturial Kid
The Acturial Kid

Reputation: 232

your string reverse function is incorrect

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

Upvotes: 1

Konrad Rudolph
Konrad Rudolph

Reputation: 545588

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.

Upvotes: 8

Adrian Mole
Adrian Mole

Reputation: 51825

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.

Upvotes: 6

user5550963
user5550963

Reputation:

Your problem is here:

int string_size = text.size(); // equals 

...
while (string_size >= 0) {
   ....
}

if you pass "Hello" to your reverse_string() function you string_size would be 5. Your loop would go in for 5, 4, 3, 2, 1, 0. You have two solutions, decrement string_size or do while (string_size > 0) { instead of >= 0. Later one would also need adjustment here reversed += text[string_size]; should be reversed += text[string_size - 1];

Upvotes: 3

Related Questions