0

I need to find the maximum remainder for n divided by any integer number from 1 to n, and the denominator which this remainder is found with.

In my implementation fun1 works as expected and returns the max remainder, fun2 is supposed to give 3 but its giving 2 .probably mistake is at break statement.

Sample input: 5

Expected output: 2 3.

My output: 2 2.

#include <iostream>
#include <algorithm>

using namespace std;

int fun2(int a);
int fun1(int n ,int num);

int main(){
    int n = 0; int num = 0;;
    cin >> n;
    int p = fun1(n, num);
    cout << p << "\n";
    cout << fun2(p);
}

int fun1(int n, int num){
    int b = 0;
    
    for(int i = 1; i <= n; i++){
        num = n % i;
        b = max(num, b);
    }
    
    return b;
    
}

int fun2(int n,int p ){
     int num = 0; int c = 0; int d = 0;
    for(int i = 1; i <= n; i++){
        num = n % i;
        c = max(num, c);
        
        if(c == p){
            break;
        }
        d = i;
    }
    return d;
}
Anirudh
  • 3
  • 3
  • How's it look inside a debugger? – tadman Jul 27 '22 at 13:37
  • It sounds like you may need to learn how to use a debugger to step through your code. With a good debugger, you can execute your program line by line and see where it is deviating from what you expect. This is an essential tool if you are going to do any programming. Further reading: [How to debug small programs](http://ericlippert.com/2014/03/05/how-to-debug-small-programs/) and [Debugging Guide](http://idownvotedbecau.se/nodebugging/) – NathanOliver Jul 27 '22 at 13:38
  • There are no iterators in this code. If there were, you could probably avoid looping through the entire array twice. – sweenish Jul 27 '22 at 13:41
  • A side note: better to avoid `using namespace std` - see here https://stackoverflow.com/questions/1452721/why-is-using-namespace-std-considered-bad-practice. – wohlstad Jul 27 '22 at 13:44
  • `sample input :5` -- If that's the sample input, then you should simply set `n = 5;` and not use any input, i.e. `cin` statements, and then post that code. That way, others can compile and run the program, see the issue, and not need to enter data each time they run the program. – PaulMcKenzie Jul 27 '22 at 13:52
  • Maybe it is because 1 is not less than or equal to 0. – Jim Rhodes Jul 27 '22 at 13:52
  • You start with `int n = 0;`, so how many times does the loop `for(int i = 1; i <= n; i++)` execute? – molbdnilo Jul 27 '22 at 14:04
  • You never use the value of the `num` parameter for `fun1`. Why is it a parameter? – molbdnilo Jul 27 '22 at 14:06
  • yes the mistake is in not giving n as an argument to func 2 and hence it is referring n as 0,thank you all for other sugggestions regarding the code – Anirudh Jul 27 '22 at 15:17

3 Answers3

1

Since you already managed to successfully find the biggest remainder, you may get use of this function and return the number this remainder is found with:

std::pair<int, int> biggestRemDem(int value) {
    int dm = 1;
    int rm = 0;
    
    for(int i = dm; i <= value; ++i){
        const auto tmpRm = value % i;
        if (tmpRm > rm) {
            rm = tmpRm;
            dm = i;
        }
    }
    
    return { rm, dm };
    
}

The signature of the function needs to return std::pair however, but you no longer need the std::max, so the headers required to include are also changed:

#include <iostream>
#include <utility>

std::pair<int, int> biggestRemDem(int value);

int main(){
    int n{};
    std::cin >> n;
    const auto result = biggestRemDem(n);
    std::cout << result.first << " " << result.second << std::endl;
}
arrowd
  • 33,231
  • 8
  • 79
  • 110
The Dreams Wind
  • 8,416
  • 2
  • 19
  • 49
0

I need to find the maximum remainder for n divided by any integer number from 1 to n, and the denominator which this remainder is found with.

It seems that the asker decided to solve this in two steps. They wrote a function fun1 returning the maximum remainder and a function fun2, which fed with the previously calculated remainder, returns the corresponding dividend.

While not an efficient approach, it could work if implemented correctly, but that's not the case.

Other than some (very) bad naming choices, we can find:

  • In the original version of the posted code, fun2 has a function prototype with a single parameter and it is called passing the value returned by fun1, which is the maximum remainder. The problem is that this way the function has no way to know what was the original value of n and actually declares a local n, initialized to zero, so that the body of the loop for(int i = 1; i <= n; i++) is never executed.
    The actual version of this question shows a definition of fun2 with two parameters, that can't compile unless both the prototype and the call site are changed accordingly.

  • Assuming that the original n and the remainder p were succesfully passed to fun2, there still would be another issue:

    int fun2(int n, int p ) {
      int c = 0, d = 0;
      for(int i = 1; i <= n; i++) {
        int num = n % i;
        c = max(num, c);
    
        if(c == p){  // So, if we reach the passed remainder...
          break;     // We break out of the loop, skipping...
        }
        d = i;       // this line, meaning...
      }
      return d;      // That the dividend previous to the correct one is returned!
    }
    

    They could just return i; when c == p.

The answer by The Dreams Wind presents a much better approach to this task. I'd like to suggest an O(1) solution, though. Consider these points:

  • The result of n % i can't be equal or greater than i. It's in the range [0, i).

  • n / 2 is the greatest number that can divide n other than n itself. It means that all the numbers i in (n/2, n) are such that n % i > 0.

  • For every number i in (n/2, n), we can actually say that n % i = n - i.

So, when n is greater than 2, the i corresponding to the maximum remainder is just 1 + n/2 and said remainder is n - n/2 - 1.

Bob__
  • 12,361
  • 3
  • 28
  • 42
0

In fun2 you have:

    if(c == p){
        break;
    }
    d = i;

When you found the right index so that c == p the break will exit the loop and d == i; is not execute. Therefore d has the value from the previous loop, i.e. one less than you need.


Apart from that the code really smells:

fun1

  • should not have a second argument sum.
  • should remember the index where if found the largest remainder and you would be done

fun2

  • the maximum remainder is p, no need to max(num, c). Actually drop the c alltogether and just use num == p
  • n % 1 == 0 and n % n == 0. The loop will always break with i < n. Might as well not have a conditional: for(int i = 1; ; i++)
  • you need d because at the end of the loop i disappears. Why not pull i out of the loop? int i; for(i = 1; ; i++)
  • and now you can use a different conditional again
    int fun2(int n,int p ){
        int i;
        for(i = 1; n % i != p; i++) { }
        return i;
    }

or

    int fun2(int n,int p ){
        int i = 1;
        for(; n % i != p; i++) { }
        return i;
    }

or

    int fun2(int n,int p ){
        int i = 1;
        while(n % i != p) ++i;
        return i;
    }
Goswin von Brederlow
  • 11,875
  • 2
  • 24
  • 42