-2

When this program ran with input 63923 99999, it stopped giving the title message . Anyone please help me to figure out what i am doing wrong with this code. After checking with some assert inserting and even doing debug i can't able to figure out the problem with this code. this is my code :

    #define _CRT_SECURE_NO_WARNINGS 
    #include<cstdio>
    #include<vector>
    #include<iostream>
    #include<string>
    using namespace std;
    #define ll long long
    #define f(t,i,s,r) for(t i=s;i<r;i++)
    ll s, m;
    vector<ll>v;
    string g="     Good Choice", b="     Bad Choice";
    bool update (ll x,ll count) {
        if (count < m) {
              x = (x + s) % m;
            if (v[x] == 1) {
                return false;
            }
            else {
                v[x] = 1;
                return update (x, count+1);
            }
        }
        else {
            f (ll,i, 0, m) {
                if (v[i] == 0)return false;
            }
            return true;
        }
    }
    int main () {
        freopen ("i.txt","r",stdin);
        while (cin>>s>>m)
        {
            v.clear ();
            v.resize (m,0);
            v[0] = 1;
            if (update (0, 1) == true) {
                printf ("%10lld%10lld%s\n",s,m,g.c_str());
            }
            else {
                printf ("%10lld%10lld%s\n", s, m,b.c_str());
            }
        }
    }
user3708629
  • 153
  • 1
  • 1
  • 7
  • 4
    Start by removing these horrific Marcos, then debug your code with a debugger. Surely you will spot the issue – Kam Dec 04 '15 at 14:27
  • 1
    You can't handle 99999 recursive calls and with those values, `x = (x + s) % m;` doesn't repeat very early (they have no common factors). – molbdnilo Dec 04 '15 at 14:42
  • 1
    aaand theres the most convoluted, obfuscated piece of faulty-recursion for today! Thats it .... internet is over for today. – specializt Dec 04 '15 at 14:43
  • 1
    Your code produces a (pardon the pun) stack overflow in the update function. You need to rewrite that function using iterative approaches, not recursion. But the real question is why you couldn't detect this when it was obvious by just running it in the debugger (you are using Visual Studio due to you using that CRT macro above). You would have seen the call stack blowing out, along with the function it is being blown out on. That in itself deserves a downvote. – PaulMcKenzie Dec 04 '15 at 14:44

1 Answers1

1

Please, when posting code for others to look at, de-obfuscate it!

It appears that with the numbers chosen, the modulus operation doesn't kick in early enough to loop the number on one you've already seen, and you get recursion too deep for what your stack can handle.

This algorithm should be rather trivially convertable from recursion to iteration.

Angew is no longer proud of SO
  • 167,307
  • 17
  • 350
  • 455
  • thanks a lot Angew. After converting the update() into an iterative one it works well.But a little more to ask.What is the max level for recursion to work well enough ? – user3708629 Dec 04 '15 at 14:49
  • 1
    The `% m` "kicks in" every time x + 63,923 reaches 99,999 - `(63923 + 63923) % 99999` is 27,847. The issue is that a cycle is 99,999 additions long. – molbdnilo Dec 04 '15 at 14:51
  • @user3708629 There is no set "max level". You are to judge when or if recursion should be used, and obviously 99,999 recursive calls is too much. Recursion is great in theory and on the blackboard, but used in real programs, has a lot of limitations. If the number of recursive calls is relatively known, and it will be small, then yes, recursion may make sense. But if there is a chance there will be hundreds or thousands of recursive calls, then you would be wise to use an iterative approach. – PaulMcKenzie Dec 04 '15 at 14:55