3

I'm new to C++, and trying to go through [Project Euler][1]. I got all the way to [Problem 4][2] (impressive I know) and am having trouble with what I think is the scope of my variables inside a while loop. If you don't know, the problem asks you to find the highest palindrome product of two three digit integers. I made a while loop which should test if a product is a palindrome (which I put into another function - which works fine).

Here's my current code (although it's changed many times - I tried to make this one the most definite, which is why all the else ifs):

int main()
{

    int int1 = 999;
    int int2 = 999;
    int nProduct = int1 * int2;
    int nFinalProduct = 0;

    while (int1 >= 100)
    {
        if (paltest(nProduct) == 1 && nProduct > nFinalProduct && int2 > 100)
        {
            nFinalProduct = nProduct;
            --int2;
        }
        else if (paltest(nProduct) == 1 && nProduct > nFinalProduct
                 && int2 == 100)
        {
            nFinalProduct = nProduct;
            --int1;
        }
        else if (paltest(nProduct) == 0 && int2 > 100)
        {
            --int2;
        }
        else if (paltest(nProduct) == 0 && int2 == 100)
        {
            --int1;
        }
    }
    cout << nFinalProduct;
}

I'm basically trying to say if the product is a palindrome AND higher than the previous one, add it to nFinalProduct, and decrement int1 or int2 to get the next product.

I've tried rewriting main() several times using the same kind of logic, but each time the output doesn't change from what I initialise nFinalProduct to (in this case 0). Is it only updating the value inside the while loop and then resetting it once the loop ends? My solution for Project Euler's third problem uses the same idea of initialising a variable, changing it inside a while loop and printing it outside the loop, which works fine. I can't think of what the problem here is, except maybe if it's never finding paltest() to be 1, which I've tested heaps and cant find a problem with.

Any help is appreciated.

UPDATE: Ok guys, thanks heaps. I moved the nProduct declaration to inside the while loop, and now it wont end. This is my new code:

int main(){

    int int1 = 999;
    int int2 = 999;
    int nFinalProduct = 0;

    while (int1 >= 100){

        int nProduct = int1 * int2;

        if (paltest(nProduct) == 1 && nProduct > nFinalProduct && int2 > 100){
            nFinalProduct = nProduct;
            --int2;
        }
        else if (paltest(nProduct) == 1 && nProduct > nFinalProduct && int2 == 100){
            nFinalProduct = nProduct;
            int2 = 999;
            --int1;
        }
        else if (paltest(nProduct) == 0 && int2 > 100){
            --int2;
        }
        else if (paltest(nProduct) == 0 && int2 == 100){
            int2 = 999;
            --int1;
        }
    }
    cout << nFinalProduct;
}

Which now will just run indefinitely. My feeling is that int1 is never being decremented (which would eventually terminate the loop). If it's not being decremented, it means that int2 is never decrementing. Am I on the right track?

[1] https://projecteuler.net [2] https://projecteuler.net/problem=4

H.S.
  • 11,654
  • 2
  • 15
  • 32
Daniel Forsyth
  • 313
  • 2
  • 4
  • 15
  • It likely means none of those code paths that reassigns to `nFinalProduct` got executed which means the conditions checking them never got satisified. – greatwolf Sep 10 '13 at 02:39
  • 2
    You should really have accepted the answer that fixed this problem and started a new question. – kfsone Sep 10 '13 at 03:38

4 Answers4

0

If I understand the problem correctly, you want to update 'nProduct' in every iteration of the loop. So the only change would be to put 'nProduct = int1 * int2;' right below the 'while (int1 >= 100){'.

0

It looks like one of the problems in your code is that nProduct never gets updated in the loop. You initialized it to 999*999 outside the loop and it stays that way through every loop iteration. So you keep checking the same number.

I'll let you figure out how to fix that.

Edit: Your palindrome checking function isn't handling the general case. It's trivial to refactor it so it does:

bool isPalindrome(int nProduct)
{
  string subject = to_string(nProduct);
  for(int i = 0, n = subject.length(); i < n / 2; ++i)
  {
    if(subject[i] != subject[n - i - 1]) return false;
  }

  return true;
}
greatwolf
  • 20,287
  • 13
  • 71
  • 105
0

if you want to update nProduct than change this 'nProduct = int1 * int2;' and then it will change in every iteration

0

(Note that - I know this is very old question asked year's ago. I am just adding this answer for sake of completion as none of the other answer address the problem in second code. Not expecting any UV)

The reason for your second code doesn't end is:

While calculating, it find first palindrome for int1 = 995 and int2 = 583 values, which is 580085 and its get assigned to nFinalProduct variable.

Second palindrome is for int1 = 995 and int2 = 517 values, which is 514415 (the nProduct value is 514415).

Check your code if conditions inside while loop for int1 = 995, int2 = 517, nProduct = 514415 and nFinalProduct = 580085 values. None of the if conditions results in true, hence no change in int1 and int2 values and int1 value is 995 (>= 100) so, while loop loops forever.

Code is written in such a way that makes it unnecessary complex to read. Multiple times calling paltest() for same value, which is not required. This could be simplified like this:

int main(){

    int int1 = 999;
    int int2 = 999;
    int nFinalProduct = 0;

    while (int1 >= 100){

        int nProduct = int1 * int2;

        int res = paltest(nProduct);

        if ((res == 1) && (nProduct > nFinalProduct)) {
            nFinalProduct = nProduct;
        }

        --int2;

        if ((int2 < 100) || ((res == 1) && (nProduct < nFinalProduct))) {
            int2 = 999;
            --int1;
        }
    }
    cout << nFinalProduct;
}
H.S.
  • 11,654
  • 2
  • 15
  • 32