Finding total of prime numbers between two x1,x2, cant find mistake?

user_10515311 0
Viewed 156

I need to find the sum of prime numbers between two numbers, say x1 and x2 inclusively, however i cant detect whats wrong? for example if i entered 3 and 9 i would get 15 but i am getting 133!

#include <iostream>
using namespace std;
int prime(int n1, int n2)
{
    int  count =0;
    bool prime = true;

    for (n1; n1 < n2; n1++)
    {
        for (int i = 2; i < n1; i++)
        {
            if (n1 % i == 0) {
                prime = false;
                continue;
            }
            else
                count++;
        }
    }
    return count;

}
int main()
{
    int n1, n2;
    cout << " Enter values for n1 and n2 (n1 must be smaller than n2):   ";
    cin >> n1>>n2;


    cout << " Sum of prime numbers from " << n1 << " and till " << n2 << " inclusively : " << prime(n1, n2) << endl;
    system("pause");
    return 0;
}
function loops c++
3 Answers

Your prime function is not appropriate. This should be like. 

int prime(int n1, int n2) {
    int sum = 0;

    for (n1; n1 < n2; n1++) {       
        bool prime = true;
        for (int i = 2; i < n1; i++) {
            if (n1 % i == 0) {
                prime = false;
                break;
            }
        }
        if( prime ) { // current n1 is prime 
            sum = sum + n1;
        }
    }
    return sum;
}

You are not adding anything if your n1 is prime.

user_13390130

I am a big believer in the one loop per function. This is a very good example for this. Your inner loop checks if a number is prime so really it should be a function on its own:

bool is_prime(int n);

int sum_primes_between(int n1, int n2)
{
    int sum = 0;

    for (; n1 <= n2; n1++)
    {
        if (is_prime(n1))
            sum += n1;
    }
    return count;
}

Now not only it's easier to read, but you can better test individual parts of code. You can test that is_prime is correct and after that you can test num_primes_between is correct. If you had gone this route from the start you wouldn't even have had the bug you currently have with detecting if a number is prime.

And here is an even neater solution with range-v3:

using namespace ranges;

bool is_prime(int n);

int sum_primes_between(int n1, int n2)
{
    return accumulate(view::ints(n1, n2 + 1) | view::filter(is_prime), 0);
}
user_28053050

None of the answers provided so far respect the requirement of being inclusive. Here's a corrected version including an optimized algorithm to check for primality:

#include <iostream>
#include <cmath>

using namespace std;

bool is_prime(int n) {
    // handle special cases
    if (n <= 1) {
        return false;
    }
    if (n <= 3) {
        return true;
    }
    if (n % 2 == 0 || n % 3 == 0) {
        return false;
    }
    /* because we covered multiples of 2 and 3 we can reduce the number of checks
    drastically */
    for(int i = 5, r = sqrt(n); i =< r; i += 6) {
        if (n % i == 0 || n % (i + 2) == 0) {
            return false;
        }
    } 
    return true;
}
int sum_of_primes(int n1, int n2) {
    int sum = 0;
    // loop from n1 up to n2
    for ( ;n1 <= n2; n1++) {
        if (is_prime(n1)) {
            sum += n1;
        }
    }
    return sum;
}
int main() {
    cout<<"The sum of primes between 2 and 15 is: "<<sum_of_primes(2,15)<<endl;
    return 0;
}

Your original code didn't calculate the sum of primes, you simply incremented your count variable on every iteration that didn't find a divisor of the current number you were checking.

user_43059370
Related