0

I am beginning to learn c++, and was working through the Project Euler challenges, and #7 asks you to find all prime numbers within a given range. After online research i decided to try using Sieve of Erastothenes, however with the code i have set up, i currently get weird values such as )2, 0) when i ask for 2 primes, and (2, 4, 5, 5) when i input 5.

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

using namespace std;

int main(){

int end_point;
cout << "how many prime numbers would you like to find?\n";
cin >> end_point;

//creates a vector to store all values, that will eventually be whittled down to primes
vector<int> primes = {2};

//adds all numbers between 2 and chosen end point to the vector
for (int i = 3; i <= end_point; i++){

    primes.push_back(i);
}

for (int i = 0; i < end_point; i++){

    //starts at the first value (always 2), and feeds it into the next for loop
    //once the next loop is done, it moves on to the next value in the loop and feeds that in
    primes[i];

    //looks at values in the vector, starting with the next value in the vector
    for (unsigned int j = i+1; j < primes.size(); j++){

        //checks if the value at [j] is divisible by the value at [i]
        //if it is, this deletes it from the vecotr
        //if not, it moves on to the next value in the vector
        if(primes[j] % primes[i] == 0){

            primes.erase (primes.begin() + (j-1));
        }
        else{}
    }

//prints out all of the primes in the specified range
cout << "Primes are: ";
for (unsigned int k = 0; k <= primes.size(); k++){
    cout << primes[k] << ", ";
}
}
}
  • 2
    There are lots of bugs here, for instance you use `end_point` as if it's the size of your `primes` vector but it isn't. You will find this much easier (and your code will also be much more efficient) if you declare `primes` as a vector of booleans. So `primes[n]` is false if `n` has been proved not to be a prime. This approach means you will not have to resize the `primes` vector, which will make your code not only simpler but more efficient too. – john Jul 30 '20 at 05:39
  • 1
    `#include ` - Don't *ever* include that header. It's an internal implementation specific header, not intended for you to include and it won't work with other compilers. See also [Why should I not #include ?](https://stackoverflow.com/q/31816095/5910058) – Jesper Juhl Jul 30 '20 at 06:01
  • What is the statement `primes[i];` supposed to do? Because it doesn't. – user207421 Jul 30 '20 at 06:25
  • For Project Euler, I store primes in a `std::set`. ;-) – Caduchon Jul 30 '20 at 12:46
  • @MarquisofLorne, i intended primes[i] to keep track of where we are in the vector, for example, one the first iteration it should find the value at i = 0, which is 2, and check for multiples of 2, then when i iterates it checks at i = 1 which is 3. At least, that's what i intended. Where is the error that causes it to do nothing? – Matt Lindsey Jul 30 '20 at 15:43
  • @JesperJuhl i only included that because another website i was looking at for vectors related stuff had it in their code, thank you for the heads up! – Matt Lindsey Jul 30 '20 at 15:43
  • @john so if i were to make it a boolean, then when i print the vector, would i be able to just ask it to print the true values, or do i have to edit the vector, so the whole thing is either true/false, then print it? – Matt Lindsey Jul 30 '20 at 15:45
  • @MattLindsey Sorry but I don't understand exactly what you are asking, but yes you would just print out the true values. – john Jul 31 '20 at 06:22
  • @MattLindsey I specifically asked about the pointless *statement* `primes[i];` which accomplishes exactly nothing (except possibly trigger a SIGSEGV if `i` is way out of range). It just evaluates the expression and then throws the result away. – user207421 Aug 01 '20 at 01:13

3 Answers3

0

you can check prime from 2 to the sqrt of that number. this will omit duplicates and extra checks.

#include <iostream>
#include <math.h>
#include <vector>

using namespace std;

vector<int> getPrimes(int start, int end){
    if( start <0 || end < 0 || end < start ){
        return {};
    }
    
    int max = 0;
    bool isPrime = true;
    vector<int> prims = {};
    
    
    for( int i = start ; i <= end ; i++ ){
        max = sqrt(i);
        isPrime = true;
        for(int j = 2 ; j <= max ; j++ ){
            if( i % j == 0 ){
                isPrime = false;
                break;
            }
        }
        
        if(isPrime){
            prims.push_back(i);
        }
    }
    
    return prims;
}

int main()
{
    
    
    vector<int> prims = getPrimes(0,100);
    

    
    for(int n : prims) {
        cout << n << '\n';
    }
    
    return 0;
  
}

Sqrt Reason:

imagine you want to find out that 17 is a prime number or not. the simplest way is to loop from 0 to 17 to check its multiples. but it doesn't needed to do that. the sqrt of 17 is almost 4. so you check the multiples until the number 4. the next number is 5. from 5 to 17, all the multiples that their results is smaller that 17 is from 4 to 0. because 5 * 5 is 25 . even 5 * 4 is 20. so all multiples going to be repetitious. now you can just check multiples from 2 to square root of that number to find out the number is a prime number or not

PoryaGrand
  • 91
  • 6
  • 1
    It's correct code (I think) but it's not the Sieve of Erastothenes algorithm that the OP wanted. – john Jul 30 '20 at 06:03
  • yes. it's not, but i think he just want an algorithm to do that. – PoryaGrand Jul 30 '20 at 06:11
  • 1
    Anyone can find an implementation of _some_ algorithm on the internet. What would really help is to find and explain OPs error and help to fix them, so OP gets a better understanding of the language and the algorithm. Providing ready-to-use-solutions will often lead to people just copy-pasting code and they will run into all sorts of problems, because they don't understand the code they are using. – Lukas-T Jul 30 '20 at 06:18
  • if every one could find some algorithm on the internet , it will never needed to write these codes. OP just said ` After online research i decided to try using Sieve of Erastothenes` , so he wants help for that algorithm or a new algorithm. if he wants help for the algorithm i will be here to answer about this algorithm just if he wants to learn about it not to just copy and paste. it depends on that person and its goals. however, every one else also can use these algorithms if they needed. good luck – PoryaGrand Jul 30 '20 at 06:29
  • @churill, you're both right in the fact that i really could just find all of these on the internet if i actually looked, but that would kinda defeat the purpose, I wasn't really looking for the optimal solution or anything, i just found the sieve algorithm, thought it was interesting and decided to try my hand at that. I definitely should have specified that i preferred to use just that algorithm, and that it was just to learn. Thank you all for your help! Also, why do you only have to check up to the sqrt of the end point? – Matt Lindsey Jul 30 '20 at 15:52
0

You shouldn't remove element from array when you traverse it. You can try it by marking. example:

#include <iostream>
#include <vector>
#include <math.h>

using namespace std;

int main() {

    int end_point;
    cout << "how many prime numbers would you like to find?\n";
    cin >> end_point;

    //creates a vector to store all values, that will eventually be whittled down to primes
    vector<int> primes = { 2 };

    //adds all numbers between 2 and chosen end point to the vector
    for (int i = 3; i <= end_point; i++) {

        primes.push_back(i);
    }

    for (int i = 0; i < end_point; i++) {

        //starts at the first value (always 2), and feeds it into the next for loop
        //once the next loop is done, it moves on to the next value in the loop and feeds that in
        int val = primes[i];

        // to ensure the value is not zero
        if (val == 0)
            continue;

        //looks at values in the vector, starting with the next value in the vector
        for (unsigned int j = i + 1; j < primes.size(); j++) {

            //checks if the value at [j] is divisible by the value at [i]
            //if it is, this deletes it from the vecotr
            //if not, it moves on to the next value in the vector
            if (primes[j] > 0 && primes[j] % val == 0) {
                // set the value zero to element of array.
                primes[j] = 0;
            }
        }

        //prints out all of the primes in the specified range
        cout << "Primes are: ";
        for (unsigned int k = 0; k <= primes.size(); k++) {
            if (primes[k] > 0) // output the value which greater then zero.
                cout << primes[k] << ", ";
        }
    }

    return 0;
}
Albert
  • 29
  • 1
  • 5
0
  1. You delete wrong element. This is right:

    primes.erase(primes.begin() + j);

  2. Your last loop in wrong place. Take it out of previous 'for loop'. And you go after last element. Should be:

    k < primes.size();

not

k <= primes.size();

=== Now it works properly ===

#include <iostream>
#include <vector>
#include <math.h>

using namespace std;

int main() {

    int end_point;
    cout << "how many prime numbers would you like to find?\n";
    cin >> end_point;

    vector<int> primes = { 2 };

    for (int i = 3; i <= end_point; i++) {
        primes.push_back(i);
    }

    for (int i = 0; i < end_point; i++) {
        for (unsigned int j = i + 1; j < primes.size(); j++) {
            if (primes[j] % primes[i] == 0) {
                primes.erase(primes.begin() + j);
            }
            else {}
        }
    }
    cout << "Primes are: ";
    for (unsigned int k = 0; k < primes.size(); k++) {
        cout << primes[k] << ", ";
    }
    return 0;
}
  • The for loop probably shouldn't increment j when it removes an element – Alan Birtles Jul 30 '20 at 06:50
  • Thank you! i didnt even realize that last loop was inside of the other one. That's probably why i had a bunch of trouble, just getting it to output in the first place. @AlanBirtles, you're probably right, but in this case, i think it works out, because j has to check every other number to begin with, so skipping a value after it deletes one, ends up causing no harm as far as i can tell. – Matt Lindsey Jul 30 '20 at 15:49