patterncppMajor
Prime number generator in C++
Viewed 0 times
numberprimegenerator
Problem
I am slowly learning C and C++, and decided after a few lessons to dive in myself without any help. I developed this prime number generator. It seems fast, but I'm wondering if I'm following the best practices for C++, or if I am missing anything important.
#include "stdafx.h"
#include "stdio.h"
bool checkPrime(int Number);
int _tmain(int argc, _TCHAR* argv[])
{
int currentNum;
currentNum = 2;
do {
if (checkPrime(currentNum)) {
printf("%d ", currentNum);
}
currentNum++;
} while (1 == 1);
return 0;
}
bool checkPrime(int Number){
for (int a = 2; a < Number; a++){
if (Number % a == 0) {
return false;
}
}
return true;
}Solution
A few things in addition to what @Jamal and @200_success already wrote:
-
-
... and since you're not using the arguments of
-
As you already used
-
This may be a matter of taste, but I think
-
... actually, as @200_success pointed out, a
-
In
-
As @leetnightshade pointed out, place the opening curly either always on the same line as the function name ("Egyption brackets"), or always on the next line.
Suggested implementation
This compiles with
-
g++ on Mac OS X won't compile this code, because of #include "stdafx.h" and the int _tmain(int argc, _TCHAR* argv[]) signature. It's good to keep your code portable, unless you have a special reason not to. g++ can compile if you drop the stdafx.h import and if change the main declaration.-
... and since you're not using the arguments of
main, you could just declare without any args: int main() { ... }.-
As you already used
true in the checkPrime function, why not use it in the infinite while loop in main, instead of 1 == 1.-
This may be a matter of taste, but I think
while (true) { ... } is generally more readable and intuitive than do { ... } while (true).-
... actually, as @200_success pointed out, a
for (int currentNum = 2; ; currentNum++) { ... } loop would be even better: this way currentNum is declared in the block where it is used, eliminating potential side effects, and the counter is a natural element in a for loop. Notice the empty terminating condition, making this an infinite loop.-
In
checkPrime you named the variable int Number, but the common convention is to not capitalize variable names, use simply int number instead.-
As @leetnightshade pointed out, place the opening curly either always on the same line as the function name ("Egyption brackets"), or always on the next line.
Suggested implementation
#include
bool isPrime(int number)
{
for (int a = 2; a < number; a++) {
if (number % a == 0) {
return false;
}
}
return true;
}
int main()
{
for (int currentNum = 2; ; currentNum++) {
if (isPrime(currentNum)) {
std::cout << currentNum << " ";
}
}
}This compiles with
g++ without warnings and runs fine in Mac OS X and GNU/Linux. I would hope it works as expected in Windows too.Code Snippets
#include <iostream>
bool isPrime(int number)
{
for (int a = 2; a < number; a++) {
if (number % a == 0) {
return false;
}
}
return true;
}
int main()
{
for (int currentNum = 2; ; currentNum++) {
if (isPrime(currentNum)) {
std::cout << currentNum << " ";
}
}
}Context
StackExchange Code Review Q#46590, answer score: 31
Revisions (0)
No revisions yet.