[RESOLVED] Wrong output with pointers

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:

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

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!

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! :confused:

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

Code:

---

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!

Quote:

---

Originally Posted by leourb

Now it seems to work!

---

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

Code:

---

int len(const char* s)

---

What was wrong with the standard C-Runtime strlen?

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! :D So I had the need to write an ugly-customized copy of that!

OK. understood! :)

Thanks again to have let me think deeper about sir!

Code:

---

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].

Quote:

---

Originally Posted by leourb

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

Code:

---

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).

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

Code:

---

    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 :D)

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

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

Code:

---

#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;
}

---

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

Code:

---

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

Code:

---

int main()
{
...
}

---

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