C++ Recursion and Pointer Problems

Question

First off, this is for an assignment.

What's required:

1.Take in string input
2. copy string (stripping out white space, punctuation, and 
   covert all characters to uppercase in the process)
3. Then determine if this copied string is a palindrome.

The method required for determining a palindrome:

Base Case:  string length is <= 1
General Case: if first letter != last letter, false, otherwise
               point to next letter and write '\0' to last letter and
               call the method again

For example:

RACECAR\0   R==R
ACECA\0     A==A
CEC\0       C==C
E\0         E <= 1  TRUE!

I cannot get my isPalindrome function to work correctly. Everything else is spot on, as far as I can tell. I really think the problem lies in my recursive call. I have been debugging this for 2 days and I cannot figure why the return is wrong. Any help would be GREATLY appreciated. I'm not looking for a hand out, maybe just some extra eyes on this code. Thanks.

#include <iostream>
#include <string>
#include <cctype>   
using namespace std;

int charCount(char  * copy)
{
    int count = 0;

    for (int i = 0; copy[i] != '\0'; i++)
    {
        count++;
    }

    return count;
}

bool isPalindrome(char *copy)
{
    bool result = false;
    int size = charCount(copy);

    char * last = &copy[size - 1];

    if (size <= 1)
    {
        result = true;
    }

    if (copy != last)
    {
        result = false;
    }

    else
    {
        ++copy;
        last = '\0';
        isPalindrome(copy);
    }

    return result;
}

void stringCopy(char * source, char * destination)
{
    int sourceIndex = 0;
    int destIndex = 0;

    while (source[sourceIndex] != '\0')
    {
        while (!(isalnum(source[sourceIndex])) && source[sourceIndex] != '\0')
        {
            sourceIndex++;
        }

        if (source[sourceIndex] == '\0')
        {
            break;
        }

        if (isalpha(source[sourceIndex]))
        {
            destination[destIndex] = toupper(source[sourceIndex]);
        }

        if (isdigit(source[sourceIndex]))
        {
            destination[destIndex] = source[sourceIndex];
        }

        sourceIndex++;
        destIndex++;
    }

    destination[destIndex] = '\0';
}

int main()
{
    string input = "";

    cout << "Enter a string: ";
    getline(cin, input);

    char * source = &input[0];
    int sourceSize = charCount(source);

    char * copy = new char[sourceSize];

    stringCopy(source, copy);

    int copySize = charCount(copy);

    if (isPalindrome(copy))
    {
        cout << input << " is a palindrome!" << endl;
    }

    else
    {
        cout << input << " is not a palindrome" << endl;
    }

    return 0;
}

Answer 1

Four errors

First you have three cases, but you only want one to execute, so it should be a single if ... else if ... else ... statement, not the if ... if ... else ... statements that you have.

Second your comparison is incorrect, because you are comparing pointers not the the characters they are pointing to.

Third error is similar to the second, when you try to shorten the string you are assigning to the pointer not to the character.

Finally you forget to assign the result of the recursive call to your result variable. Quite a common newbie error.

Here's my effort (untested code)

bool isPalindrome(char *copy)
{
    bool result = false;
    int size = charCount(copy);

    char * last = &copy[size - 1];

    if (size <= 1)
    {
        result = true;
    }
    else if (*copy != *last) // else if and *copy != *last, not copy != last
    {
        result = false;
    }
    else
    {
        ++copy;
        *last = '\0'; // *last not last
        result = isPalindrome(copy); // capture return value from recursive call
    }

    return result;
}

Four errors in one function might seem like quite a lot, but these are silly errors easily fixed. The overall code quality is quite good.

Now for extra credit see if you can write a version that doesn't destroy the string as it goes. Because you assign *last = '\0' you are changing the string as you work.

Answer 2

copy != last

Those variables are pointers to char. You need to dereference them before comparing.

Try: *copy != *last

Answer 3

This does not target any of the problems in your code, and does not show how to write the code according to the assignment. The code is only here for completeness, to show how you would solve the problem with the tools c++ already provides, and without reinventing the wheel.

std::copy_if and std::transform are your stringCopy and the std::equal with the backward and forward operator is basically what you do in your isPalindrome. Using recursion for this case isn't a good idea anyways.

#include <string>
#include <algorithm>
#include <iostream>


bool is_palindrom( const std::string &input ) {
    std::string copy;

    // copy everything except spaces and punctations using:
    // copy_if, isspace and ispunct
    std::copy_if(input.begin(), input.end(), std::back_inserter(copy), [] (int ch) -> bool {
        return !::isspace(ch) && !::ispunct(ch);
    });

    // transform to uppercase
    std::transform(copy.begin(), copy.end(), copy.begin(), ::toupper);

    // check if palindrom using iterators and revers iterators
    // copy.end()  - halfway is valid as std::string::iterator is a random access iterator
    size_t halfway = copy.size() / 2;
    bool isPalindrom = std::equal(copy.begin(),  copy.end()  - halfway, 
                                  copy.rbegin(), copy.rend() - halfway);

    return isPalindrom;
}

int main() {
    std::cout <<  is_palindrom("2aba2")  << std::endl; // 1
    std::cout <<  is_palindrom("2 ab a2")  << std::endl; // 1
    std::cout <<  is_palindrom("2 abb a2")  << std::endl; // 1
    std::cout <<  is_palindrom("abca")  << std::endl; // 0
}