-2

I tried solving the 3n+1 problem(UVa 100),here is my code but according to UVa online judge my program gives a wrong answer,my code passed all the test cases i can think of but unable to detect what is wrong, please help me find the mistake.

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

using namespace std;

vector<int> Num;// for Memoization till 1000000

int counter(long int j)  {
    if(j > 1000000) {
        if(j%2 == 1)  return(counter((3*j+1)>>1)+2);
        else return(counter(j>>1)+1);
    }
    else {
        if(Num[j] != 0) return Num[j];
        if(j%2 == 1)  Num[j] = counter(3*j+1)+1;
        else Num[j] = counter(j>>1)+1;
    }
}

int main() {
    Num.resize(1000001);//auto initilizes the vector with 0
    Num[1] = 1; // set the counter for n = 1 as 1;
    int X,Y;
    while ( cin >> X >> Y )  {
        int x = X , y = Y, mxl = 0;
        if(X > Y) swap(X,Y);
        for ( long int j = Y; j >= X; --j)  {
            if(Num[j] == 0)  counter(j);
            if(mxl < Num[j])  mxl = Num[j];
        }
        cout << x << " " << y << " " << mxl << endl;
    }
return 0;
}
J Ajay
  • 3
  • 1

1 Answers1

1
int counter(long int j)  {
    if(j > 1000000) {
        if(j%2 == 1)  return(counter((3*j+1)>>1)+2);
        else return(counter(j>>1)+1);
    }
    else {
        if(Num[j] != 0) return Num[j];
        if(j%2 == 1)  Num[j] = counter(3*j+1)+1;
        else Num[j] = counter(j>>1)+1;
    }
}

Where's your return value in the case where Num[j] == 0 in that last code segment (the outer else)?

You set Num[j] to the right value but never return it.

I suspect what you're after is something like (getting rid of those totally unnecessary if..return..else constructs as well):

int counter(long int j)  {
    if(j > 1000000) {
        if(j%2 == 1)
            return(counter((3*j+1)>>1)+2);
        return(counter(j>>1)+1);
    }
    if(Num[j] != 0)
        return Num[j];
    if(j%2 == 1)
        Num[j] = counter(3*j+1)+1;
    else
        Num[j] = counter(j>>1)+1;

    return Num[j];  // This is important.
}

I should mention, however, that recursion may not be the ideal tool to use here. For one, the possibility that a sequence may be very long means that you may run out of stack space before you get a solution.

Secondly, recursive calls are not without cost. A lot of time may be spent in setting up and tearing down stack frames as you search for your solution.

I would tend to prefer an iterative solution to avoid those potential issues.

paxdiablo
  • 854,327
  • 234
  • 1,573
  • 1,953
  • Why is there `>>1` after `3*j+1` ? – M.M Mar 19 '15 at 07:19
  • OK, I think it's trying to condense two steps into one – M.M Mar 19 '15 at 07:27
  • @Matt, yes, it appears so. For an odd `j`, `3j+1` is guaranteed to be even so it does `>>1` to divide that by two. That's why it has the `+2` on the end, adding _two_ steps, whereas the other ones are `+1`. – paxdiablo Mar 19 '15 at 08:20
  • @paxdiablo, Thank you, it is accepted now and the run time is 0.038. The problem i am facing in iterative solution is, too much of memory required to keep track. – J Ajay Mar 19 '15 at 14:47