Alright, this is driving me nuts and I have no idea what's going on. I have some code I'm making for school that basically takes in a string and a start and end index, and spits out true if the string is a palindrome, and false if it isn't. I added some extra stuff to make sure that it works if you give it a sentence with spaces and punctuation, and besides a small logic error that makes it return true if the middle two letters are different, it works..... almost. It has a super funny error where it seems to act differently on different compilers. On repl.it, I get a stack smash detected
message for basically any input, but for some things, like inputting Hello, World!
I get a segmentation fault. When I run it on Visual Studios 2019 it returns 24 for every false input, I don't even know how a boolean even returns something like 24, but if I change the function from type bool
to type int
, then it works perfectly.... except for repl.it, where I still get stack smashing. I also don't want to just leave it at an int and call it a day because that feels like a bandaid, what the heck is going on??
tldr; Boolean function returning 24 on Visual Studios, and stack smashing on others, changing the function return type to int instead of bool fixes it on Visual Studios, but not for others.
#include <iostream>
#include <string>
using namespace std;
bool isPalindrom(string, int, int);
int main() {
string str = "R aceCar";// should return true
bool thing = isPalindrom(str,0,str.length()-1);
cout << thing << endl;
}
bool isPalindrom(string str, int start, int end){
if(end - start == 1 && (tolower(str[start]) == tolower(str[end])) || start == end){
return 1;
}
else{
if((str[start] >= 'a' && str[start] <= 'z') || (str[start] >= 'A' && str[start] <= 'Z')){
if((str[end] >= 'a' && str[end] <= 'z') || (str[end] >= 'A' && str[end] <= 'Z')){
if(tolower(str[start]) == tolower(str[end])){
// cout << str[start] << ' ' << str[end] <<endl;
isPalindrom(str, start 1, end-1);
}
else{
return 0;
}
}
else{
isPalindrom(str, start, end -1);
}
}
else{
isPalindrom(str, start 1, end);
}
}
}
CodePudding user response:
you are not returning a value from isPalindrom, - your compiler is surely warning you about it, mine did
1>C:\work\ConsoleApplication1\ConsoleApplication1.cpp(68): warning C4715: 'isPalindrom': not all control paths return a value
you probably - its your logic - just want
if (tolower(str[start]) == tolower(str[end])) {
// cout << str[start] << ' ' << str[end] <<endl;
return isPalindrom(str, start 1, end - 1); <<<<===
}
else {
return 0;
}
}
else {
return isPalindrom(str, start, end - 1); <<<<====
}
}
else {
return isPalindrom(str, start 1, end); <<<<====
}
with those edits your code now says 0 - which I assume means that "R ceCar" is not a palindrome
Also this really should be
bool isPalindrome()
CodePudding user response:
pm100's answer is the guidance you seek. But Another minor bug I spotted was your exit check at the beginning of the function:
if(end - start == 1 || start == end){
return 1;
}
Consider what happens when you have this simple test:
string str = "AB";// should return false
int thing = isPalindrom(str,0,str.length()-1);
cout << thing << endl;
As you have it coded now, isPalindrom
will return 1, but you really want it to return 0 since "AB"
is not a palindrome.
There's a very simple fix. And if you have to code a complex solution for that edge case, you're doing it wrong. Let me know if you don't see the quick fix and I'll provide another hint.
And to make your recursion more efficient, you could avoid making a copy of str
on every recursion by passing it as const reference:
int isPalindrom(const string& str, int start, int end)