Project Euler #4 - Largest Palindrome Product Problem Statement:


  A palindromic number reads the same both ways. The largest palindrome
  made from the product of two 2-digit numbers is 9009 = 91 × 99.
  
  Find the largest palindrome made from the product of two 3-digit
  numbers.


This code is in C++ 11, please review my code.

#include &lt;iostream&gt;
#include &lt;math.h&gt;

using namespace std;

int palindrom(int num);

int main()
{
    auto n1 = 999u;
    auto n2 = 999u;
    unsigned int n = 0u;

    unsigned int largest = 0;

    for (; n1&gt;=100; n2--)
    {
        if (n2 &lt; 100)
        { 
            n1--; 
            n2 = 999; 
            continue; 
        }

        if (palindrom(n1*n2) == n1*n2)
        {
            cout &lt;&lt; "n1 &lt;&lt; " &lt;&lt; n1 &lt;&lt; " n2 " &lt;&lt; n2 &lt;&lt; endl;
            cout &lt;&lt; n1*n2;

            if (largest &lt; n1*n2)
                largest = n1*n2;
        }
    }

    cout &lt;&lt; "Largest" &lt;&lt; largest &lt;&lt; endl;

    return 0;
}

int palindrom(int num)
{
    int new_num = 0;
    int digit = 0;

    while (num != 0)
    {
        digit = num % 10;
        new_num = new_num*10 + digit;
        num /= 10;
    }

    return new_num;
}


Output (project Euler solution kept secret)


   906609
Your function palindrom just "reverses" your number without actually checking it is a palindrom. It would probably be more appropriate to give this function a proper name like reverse_number() and to use it in a different function which can be called is_palindrome().

Please note that a faster implementation for this could be done differently as one could stop as soon as 2 digits do not match but we can consider that this is good enough: it's easy to test and it's easy to understand how it works.

Also, you could get rid of the magic number 10. You could for instance provide the base with a default argument.

You should always try to define your variable in the smallest possible scope.

Here's what I have for the helper function :

int reverse_number(int num, int base = 10)
{
    int new_num = 0;

    while (num != 0)
    {
        int digit = num % base;
        new_num = new_num*base + digit;
        num /= base;
    }
    return new_num;
}

bool is_palindrom(int num)
{
    return num == reverse_number(num);
}




You should compile your code with all warnings activated as they can provide you good hints :

euler3.cpp:28:18: warning: unused variable ‘n’ [-Wunused-variable]




The way you are iterating is super weird. If you want to iterate over a range with n1 and iterate over another range with n2, just use two nested for loops:

for (auto n1 = 999u; n1&gt;=100; n1--)
for (auto n2 = 998u; n2&gt;=100; n2--)


Also, without any loss of generality, one can assume that n1 &gt;= n2 :

for (auto n1 = 999u; n1&gt;=100; n1--)
for (auto n2 = n1;   n2&gt;=100; n2--)




Because the values will get smaller, you can break when you find a value smaller that the one you have already found.

At the stage, my code looks like:

int main()
{
    unsigned int largest = 0;

    for (auto n1 = 999u; n1&gt;=100; n1--)
    {
        for (auto n2 = n1;   n2&gt;=100; n2--)
        {
            auto prod = n1*n2;
            if (prod &lt; largest)
                break;

            if (is_palindrom(prod))
            {
                cout &lt;&lt; "n1 &lt;&lt; " &lt;&lt; n1 &lt;&lt; " n2 " &lt;&lt; n2 &lt;&lt; " -&gt; " &lt;&lt; prod &lt;&lt; endl;
                largest = prod;
            }
        }
    }
    cout &lt;&lt; "Largest" &lt;&lt; largest &lt;&lt; endl;
    return 0;
}