why is my 3n+1 problem solution wrong?

Question

I have recently started reading "Programming Challenges" book by S. Skiena and believe or not I am kind of stuck in the very first problem.

Here's a link to the problem: 3n+1 problem

Here's my code:

#include <iostream>
#include <vector>
#include <algorithm>

using namespace std;

unsigned long calc(unsigned long n);

int main()
{
    int i, j, a, b, m;
    vector<int> temp;

    while (true)
    {
        cin >> i >> j;
        if (cin.fail())
            break;

        if (i < j)
        {
            a = i;
            b = j;
        }

        else
        {
            a = j;
            b = i;
        }

        temp.clear();
        for (unsigned int k = a; k != b; k++)
        {
            temp.push_back(calc(k));
        }

        m = *max_element(temp.begin(), temp.end());

        cout << i << ' ' << j << ' ' << m << endl;
    }
}

unsigned long calc(unsigned long n)
{
    unsigned long ret = 1;
    while (n != 1)
    {
        if (n % 2 == 0)
            n = n/2;
        else
            n = 3*n + 1;
        ret++;
    }

    return ret;
}

I know the code is inefficient and I should not be using vectors to store the data. Anyway, I still would like to know what it's wrong with this, since, for me, it makes perfect sense, even though I am getting WA (wrong answer at programming challenges judge and RE (Runtime Error) at UVa judge).

Thanks.

Answer 1

The only wrong is:

for (unsigned int k = a; k != b; k++)

Should be:

for (unsigned int k = a; k <= b; ++k)

This is because the test is inclusive of both end points. You are excluding the top end of the range.

Optional things that could make the code easier to read:

The Loop condition tidied up:

Other things I would change are:

while (true)
{
    cin >> i >> j;
    if (cin.fail())
        break;

    // STUFF
}

Can be replaced by:

while (cin >> i >> j)
{
   // STUFF
}

Simplify the inside of the loop:

Inside the loop:

if (i < j)
{
    a = i;
    b = j;
}
else
{
    a = j;
    b = i;
}

temp.clear();

Change too:

int a  = std::min(i,j);
int b  = std::max(i,j);

vector<int> temp;

Answer 2

"...you are to determine the maximum cycle length over all numbers between i and j, including both endpoints."

Answer 3

then it should be

for (unsigned int k = a; k < b; ++k)

not

for (unsigned int k = a; k <= b; ++k)