Undefined Behaviour (Palindromic Number Finder)

Hi, so I'm trying to get the highest palindromic number from the results of a 3digit multiplied by 3 digit result. Aka 1*1 - 999*999, or any combination of those numbers.

I think there's some weird undefined behaviour going on, because I swear that I saw 2 different results from the same run. The problem is, it's giving a 7 digit result (And it's not a palindromic number). 999*999 is a 6 digit number, therefore it's impossible for the result to be a 7 digit number... So something really strange is going on, and I can't see what's wrong. Here's the code:

Code:

---

#include <iostream>
#include <string>

using namespace std;

int main() {
  char * string = new char[30];
  int highest = 0;
  int temp = 0;

    for (int x = 0; x < 1000; x++)
    for (int y = 0; y < 1000; y++) {
        itoa(x * y, string, 10);
          if ( ((int)string[0] == (int)string[5]) && ((int)string[1] == (int)string[4]) )
              if ((int)string[2] == (int)string[3])
                temp = (int)string;
                if (highest < temp)
                    highest = temp;
        }
  cout << highest << endl;
}

---

Cheers.

> temp = (int)string;
What's this do?
Surely not "1234" into 1234

Also consider that for small numbers, your array access are past the end of the string, and so you're reading garbage.

Quote:

---

Originally Posted by Salem

> temp = (int)string;
What's this do?
Surely not "1234" into 1234

Also consider that for small numbers, your array access are past the end of the string, and so you're reading garbage.

---

Hmmm that's what I originally thought temp = (int)string; would do, but now when I look at it that doesn't really make sense, you're right. Now to get around this :/

You really should not be allocating your memory on the heap, because it's not necessary and you never free it. You can just as well create it on the stack.
But even so, unless really necessary, you should use std::string instead of char.
It's possible to convert strings to number through boost (best C++ solution I know of) or strtol (or some other function?).

It's all good now :) The simple solution was staring me in the face the whole time, I had to store x * y in an integer >< Instead of trying to screw around with a way to store it BACK into an integer to do comparison, I don't know why I didn't think of it earlier lol:

Code:

---

#include <iostream>

using namespace std;

int main() {

        char * string = new char[7];
        int product = 0;
        int palin = 0;

        for (int x = 1; x < 999; x++)
                for (int y = 1; y < 999; y++) {
                        product = x * y;
                        itoa(product, string, 10);
                                if ( ((int)string[0] == (int)string[5]) && ((int)string[1] == (int)string[4]) )
                                        if ((int)string[2] == (int)string[3])
                                                if (product > palin)
                                                        palin = product;
                }
        cout << palin;
        delete string;
}

---

Why do you use a string in the first place? You only perform integer operations.
Also, itoa is not really standard, so it's better to use stringstreams or sprintf.
Also, your delete string should really be delete [] string (because you use new[]).

Because I needed to store each digit of the decimal number in an array element. The reason I'm using a pointer to a new character array is because itoa takes a pointer to a character array. I could use something else. But there's not much need anymore lol ^_^ I'll keep it in mind for future reference though. Cheers for the help.

Quote:

---

Originally Posted by pobri19

The reason I'm using a pointer to a new character array is because itoa takes a pointer to a character array. I could use something else. But there's not much need anymore lol ^_^

---

You can use a local array. It works just the same, you'll see.

Quote:

---

Originally Posted by Elysia

You can use a local array. It works just the same, you'll see.

---

I tried that, it wouldn't work :(

But it does. What did you code look like?
Did you do something like:

Code:

---

char buf[30];
itoa(&buf, ...)

---

?
In such case, the correct code would be:

Code:

---

char buf[30];
itoa(buf, ...)

---

(Lost the &.)

Oh wow I'm an idiot LOL, I did:

Code:

---

char * string[7];

---

I forgot to take out the pointer when I backspaced the new ;p haha. Yeah it works lol, thanks! :D

Just be sure to not use numbers > 999 999, or you'll get buffer overruns!
You can, of course, increase the space of the array. The biggest number a 64-bit can hold is 18 446 744 073 709 551 615 (20 digits). So an array of 21 elements or bigger should keep you safe.

Your comparisons are still comparing past the end of the string.

Eg.
1*1 = 1

Which in a string is
"1\0"

Comparing past string[2] is garbage data.