segmentation fault, recursive palindrome

[DragonReborn225]

hey guys, I'm having some difficulty with finding out whether a user inputted string is a palindrome. It doesn't have to identify pure palindromes either, just the basics. Everything was going great, and just as it was looking like I'd get to bed before 3 am, I got hit with a segmentation fault. I messed around with it alot, and bypassed that error, but now it cuts a letter off the user inputted string.

for example:

say I input ffaff
It will read faff, and tell me no palindrome exists.

I have a feeling it has something to do with the arrays, as I'm not very knowledgeable on that front because we've yet to cover them in class. Thanks in advance!!!

#include <iostream>

using namespace std;

bool Palindrome(string pal, int index, int length)
{
int total; 

pal[index] = 30;

if ((length == 0 || length == 1))
{
cout << pal << ", is infact a palindrome." << endl;
return true;
}
 if (pal[index] == pal[length - 1 - index])

return Palindrome(pal, index++, length);

else
{
cout << pal << ", is infact not a palindrome." << endl;
return false;
}
}

int main () {

string word;

cout << "Enter your prospective palindrome." << endl;
cin >> word;
Palindrome(word, 0, word.length());

return 0;
}

[Alex Edwards]

Here's something I managed to whip up thanks to your logic--

#include <iostream>
#include <string>
#include <vector>

using std::cout;
using std::cin;
using std::endl;
using std::flush;
using std::string;
using std::vector;

bool isPalindrome(const string&);
const char* result(bool);

int main(){

    vector<string> values;

    values.push_back("mom");
    values.push_back("ffaff");
    values.push_back("dad");
    values.push_back("nonPalindrome");
    values.push_back("blah");
    values.push_back("A");
    values.push_back("");

    for(vector<string>::iterator i = values.begin(); i != values.end(); i++)
        cout << (*i) << " is a Palindrome? - " << result(isPalindrome((*i))) << endl;

    cout << "\n\nEnter a word to be determined as a palindrome" << endl;

    values[0].clear();
    cin >> values[0];
    cout << values[0] << " is a Palindrome? - " << result(isPalindrome(values[0])) << endl;

    cin.ignore();
    cin.get();
    return 0;
}

/**
 * Returns true if the argument string is a palindrome, returns false otherwise
 */
bool isPalindrome(const string& arg){

    /*
     * LOGIC:
     *      -If string length is 0 or 1
     *          -string is a palindrome, so return true
     *      -Else
     *          -If first and last indice of said string have the same char
     *              -generate a new string that is a substring of this string
     *               discluding the first and last indice of the previous string
     *          -Else
     *              -return false
     */
    string temp (arg);

    if(temp.length() == 0 || temp.length() == 1)
        return true;
    else{
        if(temp[0] == temp[temp.length() - 1]){
            string next = temp.substr(1, temp.length() - 2); // substring from indice 1 and up to (length() - 2) from 1
                                                             // i.e. - ffaff, f is at [1] and length is 5, so from 1 go up to 2+1
                                                             // because even though length() - 2 is 3, we go up to 3 but not at 3
                                                             // so only 2 is added
            return isPalindrome(next); // checking to see if the parsed string is a palindrome or not
        }else return false;
    }
}

/**
 * Convenience method for returning a stringified version of a boolean argument.
 */
const char* result(bool arg){
    return arg ? "TRUE" : "FALSE";
}

--though it's probably not nearly as efficient as your implementation.

What you may want to consider is recursively calling your function by pre-incrementing index and pre-decrementing the length so you can simply check the characters of a string based on the parameters of your method and not have to mess around with arrays much.

It's just a suggestion, but I think you'll find things much easier that way =)

[chococrack]

#include <iostream>

using namespace std;

bool Palindrome(string pal, int index, int length)
{

	while(index < length)
	{
		if (length == 0 || length == 1)
			return true;
		
		if (pal[index] != pal[length - index])
			return false;
		index++;
	}

	return true;

}

int main () {

	string word;

	cout << "Enter your prospective palindrome." << endl;
	cin >> word;
	bool result = Palindrome(word, 0, word.length()-1);
	cout << word << "is";
	if(!result)
		cout << "NOT";
	cout << " a Palindrome." << endl;
	return 0;
}

And for the record, I love the "Correct indentation" function of both Notepad++ and Eclipse Ganymede <3<3<3

[DragonReborn225]

lmao sorry about the indentation, and THANKS GUYS!!!!!!!!!!

[vmanes]

What's the purpose of pal[index] = 30; ? You're changing the first character, thus making a test always fail?

Two things missing from your logic:
- The initial test of length == 0 only works on first call. Subsequent recursive calls must compare your shortened "word" length, which really should indicate index of character being compared, not actual string length. So, something like length-index== 0 is what you need.

- When making the recursive call, you must move in both the starting and ending points (index and length), and you must use preincrement/predecrement operators. Using post increment, as you do for index++, send the current value to the next call, not the update that you want to send.

Chococrack - your solution is iterative, while the OP seems to want/need a recursive solution. That said, your loop does twice the work it should, try while ( index < length/2 ) , and your line 13 has the infamous OBO error. if (pal[index] != pal[length - index -1]) solves that - remember that length is count of items, last valid item in a zero-based counting is length-1.

[vmanes]

And for the record, I love the "Correct indentation" function of both Notepad++ and Eclipse Ganymede <3<3<3

Visual C++ can also automatically indent, and correct code that's gotten out of whack. Select the code, use ctrl-k f. I don't care for the exact way it handles switches, but it does the bulk of the work fine.

[DragonReborn225]

The reason for the index = 30 thing was because I was getting segmentation faults, and I figured there wasn't enough space available or something after a quick google of those faults.

And maybe its just late, but I have no idea what you mean by preincrement/predecrement operators. Could you please give an example if your still around? Thanks.

edit: I'm thinking you mean index = index + 1, but isn't that the same thing as index++, so I guess I still don't know what you mean lol.

[vmanes]

return Palindrome(pal, index++, length) when this call is made, the current value of index is passed, then index is incremented (made larger by 1)

Putting the ++ after the variable, as you've done, is postincrementing - use the value, then increment it. Putting the ++ in front of the variable preincments - changes the value, then uses the new value. In order to pass the value of (index+1) to the recursive function call, you need the preincrement.

For that matter, you could have clearly shown intent (and not gotten any possible confusion) by writing the call as return Palindrome(pal, index+1, length-1) Try this:

int x = 5;
cout << x << endl;    //shows 5
cout << x++ << endl;   //still shows 5
cout << x << endl;  //now we see 6

cout << ++x << endl;  //shows 7

[DragonReborn225]

thanks for the added help man. Just one question though, whats the point of post-incrementing if pre-incrementing is instantaneous? also did you see any tell tale signs that led to my segmentation faults?? thanks again, and no matter if you didn't see any, you've wasted enough time on this already lol.

edit: working perfectly!!! thanks everyone.

[chococrack]

Visual C++ can also automatically indent, and correct code that's gotten out of whack. Select the code, use ctrl-k f. I don't care for the exact way it handles switches, but it does the bulk of the work fine.

I have a tendency to work away from Visual C++, no idea why. I think its not aesthetically pleasing enough for me and I find it a bit cumbersome.

I usually work with Notepad++ for small pieces of code, Eclipse Ganymede for usual everyday projects.

I know what you mean about the switch statements, it seems to be universal in the way it gets indented.

switch(num)
{
case 1: 
{
}
case 2:
{
}
default:
{
}
}

so ugly.

[vmanes]

thanks for the added help man. Just one question though, whats the point of post-incrementing if pre-incrementing is instantaneous? also did you see any tell tale signs that led to my segmentation faults?? thanks again, and no matter if you didn't see any, you've wasted enough time on this already lol.

edit: working perfectly!!! thanks everyone.

Pre- versus post-incrementing - most of the time it's a matter of style, what the coder prefers. When used in a standalone manner ( nothing else occurring in the statement), it doesn't matter.

x++ ;
//or
++x;

will give exactly the same results. On the other hand, there's a difference in

p = x++;
//or
p = ++x;

The question is, what do you really mean to occur? This is an easy way to get OBO errors (Off By One) if you don't use the version that correctly implements your algorithm. In code like this, I advise students to separate the increment/decrement from the other actions, that way it's perfectly clear when you want the change to occur.

As I mentioned earlier, to be absolutely clear what you're doing in your function call, just use index-1 as the argument, as you're not really seeking to change the value of index, just passing a value that is one less.

I didn't get any seg fault or other runtime error with your original code, just incorrect result.

Time is never wasted when it's spent helping someone learn something.
(hmm, might make that my new sig)