Trying to find all prime numbers in a list

Question

The program is checking every number in the list and should output if is prime or not. Probably I just don't get the prime algorithm right. Currently outputs that no number is prime.

void primList(struct node* stnode)                                                
{
    struct node* pr = stnode;
    int i;
    bool primNum = true;

    if (stnode != NULL)
    {
        while (pr != NULL)
        {
            if ((pr->num) == 0 || (pr->num) == 1)
                primNum = false;
            else
            {
                for (i = 2; i <= ((pr->num) / 2); ++i)   //The mistake
                    if (((pr->num) % i) == 0)            //is here?
                    {
                        primNum = false;
                        break;
                    }       
            }
            if (primNum)
                cout << "\n" << pr->num << " is a prime number.";
            else
                cout << "\n" << pr->num << " is not a prime number.";        
            pr = pr->nextptr;
            bool primNum = true;
        }
    }
}

Did you try to step through your code and see why it does not output anything? — Quimby, Dec 01 '20 at 09:08
... and if not it seems like a perfect time to learn [what a debugger is and how it can help you](https://stackoverflow.com/questions/25385173/what-is-a-debugger-and-how-can-it-help-me-diagnose-problems) — Lukas-T, Dec 01 '20 at 09:09
What is the stnode passed in when this is called? The for loop should stop at < pr->num. pr->num % pr->num will always be 0. — MAK, Dec 01 '20 at 09:09
The problem is in `bool primNum = true`. It doesn't reset the primNum variable (as I guess it's supposed to), thus if the first number is not prime, primNum is always false. Changing this line to `primNum = true` would probably fix it. — Maksymilian Mika, Dec 01 '20 at 09:11
Apart from the other issues, why are you iterating up to `num/2`? Iterating up to `√num` would make more sense. — Mike Vine, Dec 01 '20 at 10:52

score 1 · Accepted Answer · edited Dec 01 '20 at 09:43

In general such questions (seeking debugging help) are discouraged, see on-topic.

In this case the minimal change to fix this code would be:

void primList(struct node* stnode)                                                
{
    struct node* pr = stnode;
    int i;
    bool primNum = true;

    if (stnode != NULL)
    {
        while (pr != NULL)
        {
            if ((pr->num) == 0 || (pr->num) == 1)
                primNum = false;
            else
            {
                for (i = 2; i <= ((pr->num) / 2); ++i)   //The mistake
                    if (((pr->num) % i) == 0)            //is here?
                    {
                        primNum = false;
                        break;
                    }       
            }
            if (primNum)
                cout << "\n" << pr->num << " is a prime number.";
            else
                cout << "\n" << pr->num << " is not a prime number.";        
            pr = pr->nextptr;
            primNum = true; // <-- notice I erased bool here
        }
    }
}

The problem here is that the primNum variable is not really changed in line bool primNum = true, but a new variable with the same name is created (and then immediately destroyed).

There are many other ways on how your code could be improved. See for example The Definitive C++ Book Guide and List as it has a lot of great sources.

Trying to find all prime numbers in a list

1 Answers1