CODEWITHSUNDEEP
c++local-variables
Steven
Steven

Reputation: 345

local variable issue in c++

I just started learning the very basics of c++, I have experience with vb.net and c#

Now I thought to get the basics covered some Project euler problems would be a good excercise. I ran into a very strange problem by trying to solve Problem 4, probably because of a lack of understanding of c++ yet.

I got 2 scenario's, one is working, one is not(> the a variable just jumps from 999 to 768 when the sprintf command is executed as shown below), now my question is: Why is this happening in solution 1? The only difference is the loop variables a and b are declared before the loop as in solution 2 they are declared with the loop.

Solution 1 (not working):

int a =999;
int b =999;
int lb = 100;
int c = 0;
char strNumber[6];
int result = 0;
int ra = 0;
int rb = 0;
for (a=999; a>=100; a--){
    for (b = a; b>=lb; b--){
        c = a * b;
        sprintf(strNumber, "%d", c);
        if (isPalindrome(strNumber)){
            if (c > result){
                result = c;
                lb = b;
                rb = b;
                ra = a;
            }
            b = 0;
        }
    }
}

Solution 2 (working):

int lb = 100;
int c = 0;
char strNumber[6];
int result = 0;
int ra = 0;
int rb = 0;

for (int a=999; a>=100; a--){
    for (int b = a; b>=lb; b--){
        c = a * b;
        sprintf(strNumber, "%d", c);
        if (isPalindrome(strNumber)){
            if (c > result){
                result = c;
                lb = b;
                rb = b;
                ra = a;
            }
            b = 0;
        }
    }
}

Upvotes: 1

Views: 135

Answers (2)

Sarfaraz Nawaz
Sarfaraz Nawaz

Reputation: 361412

Actually both has same problem, namely undefined behavior. The problem stems from here:

char strNumber[6];

This variable cannot accomodate the value of 999 x 999 in string representation, bevause there is also a null character '\0' at the end. So when you call sprintf to convert the integer into its string representation, right there inside the function it invokes undefined behaviour when it tries to put the character'\0' at index 6, because the indices 0 to 5 are already occupied by six digits of 998001 which is the product of 999 x 999.

You should define it at least of size 7:

char strNumber[7]; //one char for null-character.

As for why your second code is working is because you're being lucky (rather unlucky). That is what undefined behavior means: anything could happen. It may run, or may not. Neither the language specification nor the compiler gives any guarantee of the behavior, hence undefined behavior.

As @James Kanze pointed out in the comment that the real solution is to forget about sprintf, and better use std::ostringstream and std::string as:

#include <sstream> //for std::ostringstream 
#include <string>  //for std::string 

std::ostringstream ss;
ss << number;
std::string s = ss.str(); //get the string representation of the number

Upvotes: 4

Mat
Mat

Reputation: 206689

The first time through the loop, c is going to be a 6-digit number (999²).
The sprintf(strNumber, "%d", c); will thus write 7 chars to strNumber: the six digits and the famous null terminator. Since you only reserved six chars for strNumber, that last write will clobber something.

It happens not to appear to clobber something important in your second case, but that's just "luck", your code's behavior is undefined in both cases.

Reserve an extra char in strNumber and both versions should work the same as far as I can tell.

Upvotes: 3

Related Questions