Can you Help a brother ? Prime Function

[WalterCC]

Hello Hello ! Is first time for me, I just sign in to this forum.
Can someone show me the code of this in C++ :
-I want a function which show me the first prime number.
1.For exemple I type 14 (N=14) then to show me the next prime number "only".
So I type 14,the prime number after this is 17. Haw you do that in a noob style without "bool,true", i am not that advanced.
2.Another exemple. I type N=2, I want to show me the next prime number which is 3.

I have the programm in my teacher book, but when I type N=2 , it show me that first number prime is 5, which is wrong, it should show me 3.

Sorry for me english ! I am from middle Europe!

[Ganado]

Hello Walter!

Show us your code, and we can try to figure our where you're going wrong. Sounds like a fencepost error.

[WalterCC]

Thank you. I translate from my language to english. I need to remind that in book was not "cin>>N"...i think is a mistake , they may forget to put it ?

#include<iostream>
#include<math.h>
using namespace std;
int FirstPrimeNumber(int N);
int isPrime(int nr);
int main(){
int N;
cout<<"N=";cin>>N;
cout<<"result="<<FirstPrimeNumber(N);

}
int FirstPrimeNumber(int N){
int nrNow; //curent number
nrNow=N+1;
for(;;){

if(isPrime(nrNow))return nrNow;
nrNow++;
}
}

int isPrime(int nr){
int i;
for(i=2;i<=(int)sqrt(nr);i++)
if(nr%i==0)return 0;
else return 1;

}

[jonnin]

isprime is totally broken.
first, it will always return a value in the loop, not checking the value correctly.
say nr is 21 in isprime.
nr%2 == 0? Nope. else.. return true. OK, 21 (3*7) is now prime. Nope.

also, 2 is not less than sqrt 2 or 3. The loop is skipped for 2 and 3, giving undefined behavior I think since there is no return statement default.

lets fix it and clean it up a little.

remove the else keyword. and it will work, I think.

[Ganado]

You should test your isPrime function by itself. You'll see it has a bug.

Test example:

// Example program
#include <iostream>
#include <cmath>

int isPrime(int nr) {

    for(int i = 2; i <= (int)sqrt(nr); i++)
        if(nr % i == 0)
            return 0;
        else
            return 1;
}

int main()
{
    for (int i = 10; i <= 20; i++)
    {
        std::cout << "isPrime(" << i << ") --> " << isPrime(i) << "\n";
    }
}

isPrime(10) --> 0
isPrime(11) --> 1
isPrime(12) --> 0
isPrime(13) --> 1
isPrime(14) --> 0
isPrime(15) --> 1
isPrime(16) --> 0
isPrime(17) --> 1
isPrime(18) --> 0
isPrime(19) --> 1
isPrime(20) --> 0

The problem is, you are always returning after the very first iteration in isPrime, no matter what. It makes the loop effectively useless.

It's also possible for your function to not return a value at all if the for loop is not entered; this is undefined behavior.

 In function 'int isPrime(int)':
12:1: warning: control reaches end of non-void function [-Wreturn-type]

To know for certain if a number is prime, you can't just check if it's not divisible by the first number you check against. You have to make sure it isn't divisible between an appropriate range of numbers between 2 and its square root.

In other words, you know it's not prime as soon as it's divisible by a number other than 1 and itself.
But you only know if it is a prime once you check it against multiple numbers.

You have to move your return true; to the end of your function; only return true if its iterated across all relevant numbers, and has found no factors.

PS:
<math.h> is C header, not a C++ header.
Prefer #include <cmath>

[WalterCC]

Really big thank you guys. I change the isPrime definition as well, is work better now.
If I type 2 it will give me the prime number 3. Yet if i type 14 it will give me 15.
Let me show you a haw it works now. It work fine till I press 14.
N=14 - result= 15
N=15 - result=17
N=16 - result=17
N=17 - result=19
N=18 - result=19

It still give me as prime numbers 15,19, maybe even more if I get more far.
I did change some things to the code, as you guys told me...I don`t know if I get exactly what you meen to say....sorry for me spending your time.

Here the New Code :

#include<iostream>
#include<cmath> //looks like I dont need it anymore,But Thx for P.S:
using namespace std;
int FirstPrimeNumber(int N);
int isPrime(int nr);
int main(){
int N;
cout<<"N=";cin>>N;
cout<<"result="<<FirstPrimeNumber(N);

}
int FirstPrimeNumber(int N){
int nrNow; //curent number
nrNow=N+1;
for(;;){

if(isPrime(nrNow))return nrNow;
nrNow++;
}
}

int isPrime(int nr){
int i;
for(i=2;i<=nr/2;i++)
if(nr%i==0)return 0;
else return true;

}

What else should I change for proper working ? I would normally ask the theacher....but he is more far with his teaching, so I cant ask him someting which I should had learn 5 months ago......#I blame Call of Duty - Ps3 for that...lol.

[doug4]

@jonnin said:

remove the else keyword. and it will work, I think.

@Ganado said:

The problem is, you are always returning after the very first iteration in isPrime, no matter what. It makes the loop effectively useless.

You said:

I cant ask him someting which I should had learn 5 months ago......#I blame Call of Duty - Ps3 for that...lol.

Two people here told you how to fix your isPrime() function. All you did was change sqrt(nr) to nr/2 (which only make the function less efficient, and does not fix the problem) and 1[code] to [code]true (which does nothing).

Instead of playing Call of Duty, you should read and try to understand the replies you have received. If you don't understand them, then ask more questions.

Let's look at your function with better formatting. I added curly braces to make the logic easier to read. (Note: please use code tags when you post code. Click the Format button that looks like "<>", and paste your code between the tags that are generated.)

int isPrime(int nr)
{
    int i;
    for(i=2;i<=nr/2;i++)
    {
        if(nr%i==0)
        {
            return 0;
        }
        else
        {
            return true;
        }
    }
}

See that the first time through your for loop, you check to see if nr is divisible by i (2 in the first case). Based on this result you either return 0 or true.

Instead, remove the else keyword and move the return true; to outside the for loop. (In your code, because you have no {}, simply removing the else takes care of this.)

By the way, if you are playing so many video games that you are 5 months behind in a programming class, you have major problems. Programming is something that takes a lot of hands-on time. You might want to go to your teacher and ask for a tutor who can focus directly on you rather than come to an on-line forum to try learn 5 months of c++ in the last few weeks before your class ends.

[WalterCC]

What can I say, ...you helped me more then you know. I stop playing games. I wasnt playing that much, you know...when I wanted to learn, my stomach was feel like burning, it was making me sick, I am more on spiritual sight, I like staying inside ..., when I disturb that inside calm with studying It just make me sick, and with Black Ops2 in specially , I did not have any worry or sick stuf. I was usually learn a lot last year in 12-class(High School), I guess I am getting older, and lazyer...but I will fix it....cause I do not wanna work...lmaao...I rather be a engineer. Sorry for me english.