Check a String is Palindrome or not. C++

Question

What is wrong in this?

#include<bits/stdc++.h>
using namespace std;

bool isPalindrome(string str)
{
  char temp[1000];
  int len=str.length();
  int i=0;
  while(len)
  {
    len--;
    temp[i++]=str[len];
  }
  temp[i]='\0';
  if (strcmp(str,temp)==0)
     return true;
  else
     return false;
 }

Why do you think there is something wrong? Please provide enough details so that the next person with the same problem can find this question and benefit from it. *You should also provide enough detail to distinguish your question from [these](https://stackoverflow.com/search?q=%5Bc%2B%2B%5D+string+palindrome).* — JaMiT, May 21 '22 at 16:00
`bits/stdc++.h` is not a standard C++ header. Don't use it. Ever. — Pete Becker, May 21 '22 at 16:06
Tip: you don't need to copy the string. Think how you'd do it if the text was written on a piece of paper. — Pete Becker, May 21 '22 at 16:08
we sometimes need to focus on the question .. he ask what he did wrong and not how to right it better :) — Yanshof, May 21 '22 at 16:11
Just about everything. `#include` should never be used, `using namespace std;` should never be used, `temp` is totally unneeded, 1000 is a meaningless arbitrary value, `strcmp` is not applicable to `std::string`. All of this can be done in one line of idiomatic C++. — n. m. could be an AI, May 21 '22 at 16:17
You could use `std::reverse` to reverse a `std::string` and then compare the original and reversed version for equality. Less than a handful of code lines to do (and there are other equally simple options). This is way too complex compared to what's needed with modern idiomatic C++. — Jesper Juhl, May 21 '22 at 16:37

score 5 · Answer 1 · answered May 21 '22 at 15:57

Here's how you do it:

#include <string_view>
#include <algorithm>

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

Highlights:

std::equal
std::string_view
std::string_view::rbegin

DimitrijeCiric · Answer 2 · 2022-05-21T15:56:25.120

2

Your code is far more complex than it should be.

bool isPalindrome(const string& str) {
    int i = 0, j = str.size() - 1;
    while (i < j) {
        if (str[i] != str[j]) return false;
        i++, --j;
    }

    return true;
}

Another implementation:

bool isPalindrome(const string& str) {
    for (int i = 0; i < str.size() / 2; i++)
        if (str[i] != str[str.size() - i - 1])
            return false;
    return true;
}

strcmp() function that you are using, accepts only c-string as arguments, but you are passing C++ string. Also it won't work if string str length is more than 1000.

edited May 21 '22 at 15:56

answered May 21 '22 at 15:49

DimitrijeCiric

446
1
10

The first one will do bad things when `str` is empty. – Pete Becker May 21 '22 at 16:09
1

What bad things? @PeteBecker – DimitrijeCiric May 21 '22 at 16:12
@DimitrijeCiric Well, try it with an empty string and you'll see – Aykhan Hagverdili May 21 '22 at 16:13
1

@AyxanHaqverdili I tried, everything is correct. Returns 1, that is what it should return. – DimitrijeCiric May 21 '22 at 16:15
2

I'm still not sure what's supposed to go wrong in the empty `str` case. `str.size() - 1` is a huge positive unsigned number, but then it gets assigned to an `int` and ends up being `-1`, so the `while` loop is never entered and `true` is immediately returned. What's the catastrophe? – Nathan Pierson May 21 '22 at 16:15
@DimitrijeCiric `str.size() - 1` becomes a huge positive number, which overflows the `j`, which is, formally, undefined behavior. – Aykhan Hagverdili May 21 '22 at 16:16
@NathanPierson it's undefined behavior... – Aykhan Hagverdili May 21 '22 at 16:17
2

@AyxanHaqverdili It's [implementation-defined until C++20](https://en.cppreference.com/w/cpp/language/implicit_conversion#Integral_conversions) and the expected value according to modular arithmetic since then. Not UB. – Nathan Pierson May 21 '22 at 16:18
@NathanPierson sure, still not good. And can be avoided with a cast to ptrdiff_t, or just using the second version. – Aykhan Hagverdili May 21 '22 at 16:20
@AyxanHaqverdili No need of casting, we can just use `size_t` (`for (size_t i{}, j{str.size()}; i < j--; ++i) { if (str[i] != str[j]) return false; }`), iterators or `std::ssize()`. – Bob__ May 21 '22 at 16:43
1

@Bob__ well, I don't get your point. Yes, there are other ways of doing this, one is even in this answer. – Aykhan Hagverdili May 21 '22 at 16:44
2

@AyxanHaqverdili The snippet in this answer shows a conversion from the type returned by `std::string::size` to `int`. I suggested *not* to perform a conversion (as you did in your answer, sticking to iterators). – Bob__ May 21 '22 at 16:49
Could someone please make me understand this line "const string& str" ? – SKD May 21 '22 at 17:08
`const string& str`, means that `str` will not be changed in that function and `&` means that it's passed by reference, so `string` is not copied. If you have `string& str` it will pass string by reference and because it's not const if you change `str` in that function it will be changed outside that function. It's just a summary of that, there is a far bigger story about that. @SKD – DimitrijeCiric May 21 '22 at 17:12

Check a String is Palindrome or not. C++

2 Answers2