-1

Among the given input of two numbers, check if the second number is exactly the next prime number of the first number. If so return "YES" else "NO".

#include <iostream>
#include <bits/stdc++.h>

using namespace std;

int nextPrime(int x){
   int y =x;
    for(int i=2; i <=sqrt(y); i++){
       if(y%i == 0){
           y = y+2;
           nextPrime(y);
           return (y);
       }
    }
    return y;
}

int main()
{
    int n,m, x(0);
    cin >> n >> m;
    x = n+2;
    if(n = 2 && m == 3){
        cout << "YES\n";
        exit(0);
    }
     nextPrime(x) == m ? cout << "YES\n" : cout << "NO\n";
     return 0;
}

Where is my code running wrong? It only returns true if next number is either +2 or +4. Maybe it has something to do with return statement.

273K
  • 29,503
  • 10
  • 41
  • 64
  • 3
    [Why should I not #include ?](https://stackoverflow.com/questions/31816095/why-should-i-not-include-bits-stdc-h) Also [Why is "using namespace std;" considered bad practice?](https://stackoverflow.com/questions/1452721/why-is-using-namespace-std-considered-bad-practice) – Some programmer dude Jul 23 '22 at 06:10
  • 2
    As a probable hint about your problem, when you call `nextPrime` recursively, what are you doing with the value it returns? – Some programmer dude Jul 23 '22 at 06:11
  • 1
    And your conditional expression in the `main` function is a bad habit as well, when doing it like that. Use a plain `if` statement instead. All in all it looks like you have been using so-called competition or judge sites to learn, and those sites are *not* any kind of learning or teaching resource (unless all you want to learn are bad habits, bad code, and sometimes even invalid code). – Some programmer dude Jul 23 '22 at 06:12
  • @Someprogrammerdude I am unable to return the proper value. Maybe the value that it will return will be checked again and since it will match it should return the proper value. I'm relatively new to this question solving, cpp and programming in general. I tried to deduce what you are trying to say with few brain cells left -> I removed the return statement in if condition. –  अंशुल Jul 23 '22 at 06:22
  • @Someprogrammerdude you are correct about that. I am trying to learn how to build logic from codeforces. I know I have bad habits in coding, I plan to improve them, but on the same plain I am unable to build a simple logic. I guess I need to solve few questions before I move on. I'll be sure to read the articles you linked. Thankyou. –  अंशुल Jul 23 '22 at 06:25
  • After removing that return from If statement its returning correct response for 31 & 37 but flunking out on 23 & 29. I'm not sure what's happening anymore. –  अंशुल Jul 23 '22 at 06:26
  • What made you think you should remove the return from the if statement? – john Jul 23 '22 at 06:30
  • 2
    Time to learn how to [*debug*](https://ericlippert.com/2014/03/05/how-to-debug-small-programs/) your programs. For example by using a [*debugger*](https://stackoverflow.com/questions/25385173/what-is-a-debugger-and-how-can-it-help-me-diagnose-problems) to step through your code statement by statement while monitoring variables and their values. Is the code really doing what's it's supposed to do? Does it follow your design, and what you've calculated on paper? – Some programmer dude Jul 23 '22 at 06:30
  • @john because I thought when you recursively call the function, it will recheck the value of y and since any prime number would not satisfy if condition, it will automatically exit of for loop and return y from the main return statement. –  अंशुल Jul 23 '22 at 06:39
  • @Someprogrammerdude Yes, they taught us about the single stepping of a program. I tried to create my own, hence all the comments. But I still couldn't find out the bug. I need to learn how to do a proper single stepping. Thanks for the links. –  अंशुल Jul 23 '22 at 06:41
  • @अंशुल Loops are never automatically exited, and what happens in one recursive call has no effect on the previous recursive call. Every function call is independent from every other function call (recursive or not). – john Jul 23 '22 at 06:51
  • On a side note: There are ways to update the `y` of the caller (by making the int parameter an int& reference). But better fix taking the return value into account. – Sebastian Jul 23 '22 at 07:20
  • 1
    Does this answer your question? [Finding a String Palindrome with a recursive function](//stackoverflow.com/q/22890946/90527) – outis Jul 23 '22 at 07:39

2 Answers2

2

Something to do with the return statement

I would say so

       y = y+2;
       nextPrime(y);
       return (y);

can be replaced with

       return nextPrime(y + 2);

Your version calls nextPrime but fails to do anything with the return value, instead it just returns y.

It would be more usual to code the nextPrime function with another loop, instead of writing a recursive function.

john
  • 85,011
  • 4
  • 57
  • 81
  • 1
    You are correct, maybe I should have used loop. And at the start of the program I thought of using a for loop. But then I got confused with what will be the terminating condition. Maybe while loop might have been better with boolean terminating condition. Meanwhile, being not well versed in recursion, I went with it. –  अंशुल Jul 23 '22 at 06:31
  • But can you please explain it to me, how come my version doesn't return the updated value of y. After all we are updating the value of y –  अंशुल Jul 23 '22 at 06:34
  • 1
    @अंशुल The most common beginner mistake when invoking a recursive function that conveys (returns) data as a result value is the failure to reap that result. Notice how your initial calling code in `main` invokes `nextPrime`, comparing the result to `m`? You're doing *nothing* with the return result in the actual function at the recursion point. Effectively the invoke of `nextPrime(y);` is worthless, since you discard the result. The "updated" value of y you asked? Realize each recursion activation gets it *own* y. – WhozCraig Jul 23 '22 at 06:39
  • 1
    @WhozCraig Yes, I do realize it now. Thanks for pointing it out. –  अंशुल Jul 23 '22 at 06:47
2

I can tell you two things you are doing wrong:

Enter 2 4 and you will check 4, 6, 8, 10, 12, 14, 16, 18, ... for primality forever.

The other thing is

       y = y+2;
       nextPrime(y);
       return (y);

should just be

       return nextPrime(y + 2);

Beyond that your loop is highly inefficient:

for(int i=2; i <=sqrt(y); i++){

Handle even numbers as special case and then use

for(int i=3; i * i <= y; i += 2){

Using a different primality test would also be faster. For example Miller-Rabin primality test:

#include <iostream>
#include <cstdint>
#include <array>
#include <ranges>
#include <cassert>
#include <bitset>
#include <bit>

// square and multiply algorithm for a^d mod n
uint32_t pow_n(uint32_t a, uint32_t d, uint32_t n) {
    if (d == 0) __builtin_unreachable();
    unsigned shift = std::countl_zero(d) + 1;
    uint32_t t = a;
    int32_t m = d << shift;
    for (unsigned i = 32 - shift; i > 0; --i) {
      t = ((uint64_t)t * t) % n;
      if (m < 0) t = ((uint64_t)t * a) % n;
      m <<= 1;
    }
    return t;
}

bool test(uint32_t n, unsigned s, uint32_t d, uint32_t a) {
    uint32_t x = pow_n(a, d, n);
    //std::cout << "  x = " << x << std::endl;
    if (x == 1 || x == n - 1) return true;
    for (unsigned i = 1; i < s; ++i) {
        x = ((uint64_t)x * x) % n;
        if (x == n - 1) return true;
    }
    return false;
}

bool is_prime(uint32_t n) {
    static const std::array witnesses{2u, 3u, 5u, 7u, 11u};
    static const std::array bounds{
        2'047u, 1'373'653u, 25'326'001u, 3'215'031'751u, UINT_MAX
    };
    static_assert(witnesses.size() == bounds.size());

    if (n == 2) return true; // 2 is prime
    if (n % 2 == 0) return false; // other even numbers are not
    if (n <= witnesses.back()) { // I know the first few primes
        return (std::ranges::find(witnesses, n) != std::end(witnesses));
    }
    // write n = 2^s * d + 1 with d odd
    unsigned s = 0;
    uint32_t d = n - 1;
    while (d % 2 == 0) {
        ++s;
        d /= 2;
    }
    // test widtnesses until the bounds say it's a sure thing
    auto it = bounds.cbegin();
    for (auto a : witnesses) {
        //std::cout << a << " ";
        if (!test(n, s, d, a)) return false;
        if (n < *it++) return true;
    }
    return true;
}

And yes, that is an awful lot of code but it runs very few times.

Goswin von Brederlow
  • 11,875
  • 2
  • 24
  • 42