What is the problem with this boolean function?

Question

I was wondering what I may have done wrong in writing this simple function which is supposed to return true if the given number is a prime, or false if not a prime.

bool isPrime(int num)
{
    if (num <= 1)
    {
        status = false;
    }
    else
    {
        for (int i = 1; i <= num; i++)
        {
            if (num % i == 0)
            {
                dividers++;
            }
        }
        if (dividers == 2)
        {
            status = true;
        }
        else 
        {
            status = false;
        }
    }
    return status;
}

Obviously, my main looks like this:

bool isPrime(int num);
bool status;
int dividers = 0;

int main() {

    isPrime(2);
    if (!isPrime)
    {
        std::cout << "Not prime" << std::endl;
    }
    else
    {
        std::cout << "Prime" << std::endl;
    }

    return 0;
}

I'm a C++ beginner and I'd really appreciate it if someone could help me there and correct my logic. Have a good day:)

you should start by explaining why you think there is something wrong. Perhaps including actual and expected output would already be sufficient — 463035818_is_not_an_ai, Jul 27 '20 at 14:56
@UmairMubeen `isPrime` is likely defined below `main`, that is below `divisors` declaration. Otherwise, the program wouldn't compile. — Daniel Langr, Jul 27 '20 at 14:58
Why are `status` and `dividers` global variables? But the actual problem is that `if (!isPrime)` ahould be `if (isPrime(2))` (and remove the superfluous call to `isPrime`). — Paul Sanders, Jul 27 '20 at 14:59
If you want to learn something, try to implement the function without global variables. I would suggest iterating from `2` to `num-1`. Then, you don't need to count the number of `%i==0` cases at all. — Daniel Langr, Jul 27 '20 at 15:05
Well, the actual output is "Prime" for whatever number I give in as num so the function always returns true which is not good. My expected output is "Prime" obviously for prime numbers and "Not prime" for non-prime numbers (including ones equal to 1 or below 0). @DanielLangr , it should check if isPrime is equal to false:) — Paweł, Jul 27 '20 at 15:08
@Paweł Yes, and the expression `isPrime` is always `true`. Learn the difference between `isPrime` and `isPrime(2)`. These are two very different things. — Daniel Langr, Jul 27 '20 at 15:13
Implementing @idclev463035818's answer should get your program working. Then I suggest you post the resulting code to [codereview.se]. I think you'll learn a lot. — Fred Larson, Jul 27 '20 at 15:13
One issue is that your are checking the even numbers for prime. You should check for the value of 3, then increment by 2; the prime numbers after 2 are all odd. — Thomas Matthews, Jul 27 '20 at 16:53

463035818_is_not_an_ai · Answer 1 · 2020-07-27T15:04:33.830

4

The immediate problem is in this two lines:

isPrime(2);
if (!isPrime)

The first line calls the function and discards the returned value. The second line converts a pointer to the function to bool. The output of your code does not depend on what you actually do in isPrime.

That is not how you call a function and use its result!

Instead you want

if (isPrime(2)) {

or

bool isP = isPrime(2);
if (isP) { ...

As mentioned in comments, there are also problems in the implementation of isPrime, but I hope this is enough to set you back on the right track.

PS: You should get rid of the global variable status. You do not need both, the return value and a global that stores the result, and if you can choose, you should definitely go for the return value.

edited Jul 27 '20 at 15:04

answered Jul 27 '20 at 15:01

463035818_is_not_an_ai

109,796
11
89
185

Thank you, and all the comments up above for all the tips and hints. I reworked the program a little bit and made it work fine at last. Also why is it a better practice to avoid using global variables and instead use local variables? I've heard many people say that.. but just why? – Paweł Jul 27 '20 at 15:39
@Paweł: See [Are global variables bad?](https://stackoverflow.com/q/484635/10077) – Fred Larson Jul 27 '20 at 15:47
@Pawel in your code it wont make a huge difference, but consider you have 10 more functions. Then without globals, if I see `bool x = IsPrime(4);` I only need to know that line and the definition of `isPrime` to understand what is going on. With globals I need to look through all the code to understand what was the global state before and after calling the function – 463035818_is_not_an_ai Jul 27 '20 at 16:04
@Paweł Try calling your `isPrime` function twice on two prime numbers (or even on the same one twice) and you'll see one bad effect of having the global `dividers` variable. That bug is fixable but it could not even occur if `dividers` was not global. – john Jul 27 '20 at 16:07

What is the problem with this boolean function?

1 Answers1