[RESOLVED] Wrong output with pointers

[leourb]

Hi everyone! I am trying to write a function that compares the original word with its reversed. My main problem, and I cannot figure out why, I am getting always a "false" return in a boolean function that compares the two words. I have checked for spaces or strange word composition but it seems all ok. Here is my code:

int len(const char* s)
{
    int n = 0;
    while (*s != '\0')
    {
        ++n;
        ++s;
    }
    return n;
}

char* reverse_word(const char* c)
{
    int n = len(c);
    //    char* p2 = new char[n + 1];
    //    char* temp = &p2[n];
    char* p2 = new char[n + 1](); // *** default initialize
    char* temp = &p2[n - 1]; // ***
    
    while(*c != '\0') *temp-- = *c++;
    
    delete[] p2;
    
    return p2;
}

bool is_palindrome(const char* c, const char* p)
{
    if (p == c) return true;
    return false;
}


int main(int argc, const char * argv[])
{
    char ar[] = {"kayak"};
    char* p = &ar[0];
    char* p1 = nullptr;
    p1 = reverse_word(p);
    cout << p << " and " << p1 << " are ";
    if (!is_palindrome(p,p1)) cout << "not ";
    cout << "palindromes." << endl;
    keep_window_open();
    return 0;
}

Thank you very much

[VictorN]

Well, if it is your homework and you have to use pointers then learn how to debug your code and debug it to understand what where and why goes wrong.
Otherwise use STL of MFC library to make your life easier!

[leourb]

Yep it is an homework. I have tried to debug it, checking scope by scope what is going on inside the code. I always try to avoid posting for help because of "no pain no gain"! But in this case I am very puzzled and I do not know what I did wrong!

[leourb]

I have solved changing my bool function in something more verbose but more specific:

bool is_palindrome(const char* c, const char* p)
{
    int n = len(c);
    for (int i = 0; i < n; ++i)
    {
        if (c[i] != p[i]) return false;
    }
    
    return true;
}

Now it seems to work!

[VictorN]

Now it seems to work!

Good!
Now the question: why did you implement your own function

int len(const char* s)

What was wrong with the standard C-Runtime strlen?

[leourb]

You are perfectly right sir and I agree with you! I have no rancors with std functions! As I partially mentioned it is an exercise in my C++ book, written by Bjarne Stroustrup, where he forbade us to use std functions to let us appreciate them more when we can use them! So I had the need to write an ugly-customized copy of that!

[VictorN]

OK. understood!

[leourb]

Thanks again to have let me think deeper about sir!

[2kaud]

if (p == c) return true;

you are comparing pointers (ie the memory addresses) which in this case will always be different - rather than the data to which they point. If you want to compare the data at these memory addresses, use strcmp(). See http://www.cplusplus.com/reference/cstring/

Your program also has other problems - eg reverse_word you are freeing memory and then returning a pointer to the freed memory!

Why write a function len() when there is the standard strlen() function? https://msdn.microsoft.com/en-us/library/78zh94ax.aspx

Why use c-style null terminated strings when you are using c++? why not use the c++ string class?

[Posted before seen previous posts].

[2kaud]

I have solved changing my bool function in something more verbose but more specific:

bool is_palindrome(const char* c, const char* p)
{
    int n = len(c);
    for (int i = 0; i < n; ++i)
    {
        if (c[i] != p[i]) return false;
    }
    
    return true;
}

Now it seems to work!

This assumes that the length of the strings pointed to by c and p are the same (or at least string length p is not less than string length c).

[leourb]

Thanks for replying 2kaud! I have already implemented code with a check in length:

    int n = len(c);
    int m = len(p);
    if (n != m) error("Lenght of the two pointers mismatch.");

I do not use std functions because of it is an homework and I cannot use them (and I would too trust me )

For the issue regarding p2 it works anyway but you are right, I made a mistake! I have fixed it

[2kaud]

Your code is quite inefficient. len() iterates over the string and then you again iterate over the string. Also accessing via a pointer is more efficient than accessing via an index (timings with MSVS2013 confirm this - previously posted to this forum).

Consider

#include <iostream>
using namespace std;

size_t len(const char* s)
{
const char *ss { s };

	while (*ss++);
	return ss - s - 1;
}

char* reverse_word(const char* c)
{
size_t n { len(c) };

char *p2 { new char[n + 1]() };

	for (char *temp = p2 + n - 1; *c; *temp-- = *c++);
	return p2;
}

bool is_palindrome(const char* c, const char* p)
{
	while (*c && *p)
		if (*c++ != *p++)
			return false;

	return (*c == *p);
}

int main(int argc, const char * argv[])
{
const char ar[] { "kayak" };
char *p1 { reverse_word(ar) };

	cout << ar << " and " << p1 << " are " << (is_palindrome(ar, p1) ? "" : "not ") << "palindromes." << endl;
	delete [] p1;
	return 0;
}

[2kaud]

You can also undertake a palindrome check without first having to reverse the word - and by only checking half of the string! Consider

bool is_palindrome(const char* p)
{
const char *e { p };

	for (; *e; ++e);

	for (const char *s = p, *m = p + (e - p) / 2; s != m; ++s)
		if (*s != *--e)
			return false;

	return true;
}

int main(int argc, const char * argv[])
{
const char ar[] { "cabbac" };

	cout << ar << " is " << (is_palindrome(ar) ? "" : "not ") << "a palindrome." << endl;
	return 0;
}

Note that if don't need argc and argv then main can be defined as

int main()
{
...
}

[leourb]

Thank you very much 2kaud, really! I have a very performant code now and I learnt something new, thanks!!