Sieve of Eratosthenes C++

Question

currently I'm working on a project, where I want to calculate all prime numbers. When I compile (MINGW Windows Comp.) the programm crashes and returns a random error number. This is the Code I've written:

http://pastebin.com/4vVnAM2v

/*
    Sieb des Eratosthenes
*/

#include <iostream>
#include <math.h>
using namespace std;

main()
{
    //variablendeklaration
    unsigned int nmax=100;
    unsigned int i,j,erg;
    bool prim[nmax];

    //Initialisieren
    prim[0]=false;
    prim[1]=false;

    //array prim[i] erstellen
    for(i=2;i<nmax;i++)
    {
        prim[i]=true;
    }




    for(i=2;i<nmax;i++) //alle Primzahlen durchlaufen
    {
        if(prim[i] == true) //auf Prim prüfen
        {
            for(j=2;j<nmax;j++) //multiplizieren und wegstreichen
            {
                erg = j * i;
                prim[erg] = false;
            }
        }
    }

    for(i=2;i<nmax;i++)
    {
        cout << prim[i] << endl;
    }


}

Unless you're prohibited - use vector instead - `vector primes;` — Wayne Werner, Oct 03 '13 at 18:37
When you start using fairly large numbers (e.g. > 1 million), this O(n^2) version of the sieve will take forever. — Zac Howland, Oct 03 '13 at 18:55

score 1 · Answer 1 · answered Oct 03 '13 at 18:31

At this point:

            erg = j * i;
            prim[erg] = false;

you are going to eventually access beyond the bounds of prim, since both i and j can have a value of up to nmax - 1, so erg will have a max value of (nmax - 1) * (nmax - 1). You need to check for this condition and break if erg >= nmax, e.g.

            erg = j * i;
            if (erg < nmax)          // if i * j still within bounds
                prim[erg] = false;   // set prim[i * j] to false
            else                     // else i * j now >= nmax
                break;               // break out of loop and try next i value

what does it mean? do I have to change the var "nmax" in the for loop? — benni, Oct 03 '13 at 18:33

score 1 · Answer 2 · answered Oct 03 '13 at 19:26

Another way to fix this problem is to avoid extra steps in your loop:

for(i=2; i < nmax; i++)
{
    if(prim[i] == true)
    {
        for (j = 2 * i; j < nmax; j += i) // state j at 2i, increment by i
        {
            prim[j] = false;
        }
    }
}

This has the effect of 1) not looping through nmax items nested (reducing your overall complexity from O(n^2) to O(n * (n/2 + n/6 + n/10 + ...) which is effectively O(n log n), and 2) not requiring an additional bounds check since you are never going out of bounds in your looping condition.

Sieve of Eratosthenes C++

2 Answers2