c++ program giving seperate inputs but few when applied multiple inputs

Question

This is a codechef beginner problem. I know it can be done in much easier ways, but I thought to try recursion here. My program is giving correct answer when I give only one input at a time, but when I run given testcases it only shows output for first two.

https://www.codechef.com/problems/CIELRCPT

#include <iostream>
#include <bits/stdc++.h>
using namespace std;
long t,p,premaining;
int counts=0;

int menus(int premaining)
{
        long k=0;
        long j=0;
        if(premaining>=2048)
        {
            counts=premaining/2048;
            premaining=premaining%2048;
            if(premaining==0)
            {
                return counts;
            }
        }
       while(k<=premaining)
        {
            j++;
            k=pow(2,j);
        } 
        if(j==1){k=1;}
        else
        {k=pow(2,j-1);}
        premaining-=k;counts++;
        //cout<<"j-1 is "<<(j-1)<<" k is "<<k<<" premaining for "<<counts<<" is"<<premaining<<endl;
        k=0,j=0;
        if(premaining!=0) {menus(premaining);}
        else{ return counts; }
}
int main() {
    cin>>t;
    while(t--)
    {
        cin>>p;
        counts=0;
        premaining=p;
        for(long i=0;i<11;i++)
        {
            long  k=pow(2,i);
            if(k==p)
            {
                cout<<"1"<<endl;
                return 0 ;
            }
        }
        long menucount =menus(premaining);
        cout<<menucount<<endl;
      
    }
    return 0;
}

`menus` does not return a value on all paths. Enable more warnings in your compiler. — molbdnilo, Mar 09 '21 at 14:01
sir pls can u elaborate i am new to programming...what do you mean by 'path' here — Pros Cons, Mar 09 '21 at 14:02
You should read [Why is “using namespace std;” considered bad practice?](https://stackoverflow.com/q/1452721/3282436) and [Why should I not #include ?](https://stackoverflow.com/q/31816095/3282436). — 0x5453, Mar 09 '21 at 14:03
You never return the value of the recursive call. Recursive functions work exactly like non-recursive functions; a function that should return a value must do that. — molbdnilo, Mar 09 '21 at 14:04
thanks ..i got that using these bring with them ambiguity problems...will try not to use them everytime....also sir i have returned the Recursive function when it premaining =0 'return counts;' also ' if(premaining!=0) {menus(premaining);} else{ return counts; } ' sir my program runs correct recursively i think cause when for test case 10 it would have to go through recursion and it gives output 2 (which is correct)...i cant understand why it is not giving o/p when all test cases are applied simultaneously ..pls help sir — Pros Cons, Mar 09 '21 at 14:15
@underscore_d sir i have used the ' ; '...can't understand nothing more — Pros Cons, Mar 09 '21 at 14:25
This is the "give change with the minimal number of coins" problem with a different backstory. Your code is way more complicated than it needs to be (it's not at all obvious how it's supposed to work), but the reason that you're only solving the first two cases is that `main` returns when the input is a power of two. Once you fix that, you will find that your program crashes when you input 3 and can start debugging. — molbdnilo, Mar 09 '21 at 14:51

score 1 · Answer 1 · answered Mar 09 '21 at 15:06

You only solve the first two cases because main returns when the input is a power of two, and the second case, 256, is a power of two.

If you fix that, for instance with continue, you will notice that your program crashes very easily (probably due to an infinite recursion).
Since locating that bug is far from trivial, note that there is a simpler algorithm than computing powers of two and counting:

Pick the most expensive item as many times as possible
Add that number to how many menus there are with the remaining total, starting with the next cheaper menu item.

Like this:

#include <iostream>

int menus(int sum, int c)
{
    return sum <= 0 ? 0 : sum / c + menus(sum % c, c / 2);
}

int main()
{
    int t = 0;
    std::cin >> t;
    while (t--)
    {
        int sum = 0;
        std::cin >> sum;
        std::cout << menus(sum, 2048) << std::endl;
    }
}

score 0 · Accepted Answer · answered Mar 09 '21 at 14:50

There are few logical errors in your code, I'll try to point them one by one-

1.) Remove line 47 because when something like this happens your main function will be stopped(return 0 stops the main function and thus program) and your program will not run for rest of the test cases. You can use boolean flag for that purpose.

2.) You don't need the for loop inside main function, below code will consider all the cases.

3.) If you understand the problem then first you always try to pick the dish with the maximum price and you want to pick it as many times as you can, so I'm writing your code in a better recursive manner(since you want to solve it recursively)-

#include <bits/stdc++.h>
using namespace std;
long t,p,premaining;
int counts=0;

// changed your function from int to void, since you are using 'counts' variable as answer you don't need to return anything
void menus(int premaining, int dish)
{
    long long price = pow(2,dish);
    // if the premaining is greater than price of current dish we pick as many dishes of this price as we can and add it to our counts-
    if(price<=premaining){
        counts+=premaining/price;
        premaining=premaining%price;
    }
    dish--;
    // only if premaining is not equal to zero we goes into another recursive call,
    // if it is equal to zero our menus function stop running because we already have calculated the answer.
    if(premaining!=0){
        menus(premaining,dish);
    }
}

int main() {
    cin>>t;
    while(t--)
    {
        cin>>p;
        counts=0;
        premaining=p;
        menus(premaining,11);
        cout<<counts<<endl;
    }
    return 0;
}

Tip: You don't need to include iostream library since you already have include bits/stdc++ and it includes everything.

c++ program giving seperate inputs but few when applied multiple inputs

2 Answers2