我正在慢慢学习C和C ++，并在几堂课后决定不做任何帮助就潜入我自己。我开发了这个素数发生器。看起来好像很快，但是我想知道我是在遵循C ++的最佳实践，还是缺少重要的东西。

评论

可以检查素数，无需从2到n循环，只需从2到sqrt（n）的奇数就足够了

#1 楼

除了@Jamal和@ 200_success之前已经写过的东西外，由于g++和#include "stdafx.h"签名，Mac OS X上的int _tmain(int argc, _TCHAR* argv[])无法编译此代码。除非有特殊原因，否则最好使代码具有可移植性。如果删除g++导入并更改stdafx.h声明，则main可以编译。
...并且由于未使用main的参数，因此可以不带任何参数地声明int main() { ... }。您已经在true函数中使用过checkPrime，为什么不在while的无限main循环中而不是1 == 1中使用它。
这可能是个问题，但我认为while (true) { ... }比do { ... } while (true)更具可读性和直观性。
...实际上，正如@ 200_success所指出的，for (int currentNum = 2; ; currentNum++) { ... }循环会更好：这样，在使用它的块中声明了currentNum，从而消除了潜在的副作用，并且计数器是for循环。请注意为空的终止条件，这使它成为一个无限循环。
在checkPrime中，您将变量命名为int Number，但是通常的约定是不使用大写的变量名，而只需使用int number。请始终将打开的花括号放在与函数名称相同的行（“括号”）上，或者始终在下一行。

建议的实现

#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 << " ";
        }
    }
}


它可以在没有警告的情况下使用g++进行编译，并且可以在Mac OS X和GNU / Linux上正常运行。我希望它也能在Windows中按预期工作。

评论

\ $ \ begingroup \ $
为了保持一致，所有功能都不应带有埃及括号，或者所有功能都不应带有埃及括号。就此而言，目前main和isPrime不一致。
\ $ \ endgroup \ $
–leetNightshade
2014年4月7日在22:08
\ $ \ begingroup \ $
@leetNightshade：什么是埃及括号。之前没有听说过。虽然我同意一般性意见。
\ $ \ endgroup \ $
–马丁·约克
2014年4月7日23:30在
\ $ \ begingroup \ $
@LokiAstari埃及方括号将花括号放在函数名称的同一行上，例如：foo（）{而不是foo（）\ n {（使用\ n表示新行，因为我不能放在其中一条评论。）
\ $ \ endgroup \ $
–活力
2014年4月8日，0：15
\ $ \ begingroup \ $
@Vality hmm，标准K＆R支撑的怪异名称。但是确实不适合C ++。
\ $ \ endgroup \ $
– jwenting
14年4月8日在12:01
\ $ \ begingroup \ $
@jwenting我不确定该术语的来源，我认为它是Java中那种括号风格的术语，而K＆R风格将毫无意义。但是，很容易猜出C中的含义。就我个人而言，我总是喜欢下一行，否则我的眼睛很难匹配成对的括号，但是在某些约定中这是一种有效的样式。
\ $ \ endgroup \ $
–活力
2014年4月8日在12:10

#2 楼



在C ++中，您现在应该分别使用std::cout和std::cin而不是printf()和scanf()。这些位于<iostream>中，您将不再需要<stdio.h>。

std::cout示例：

int number = 1;
std::cout << "Number: " << number;


std::cin示例：

int number;
std::cin >> number;



currentNum只需初始化，不声明即可分配：

int currentNum = 2;


do while循环条件应仅为1，而不是1 == 1。这仍然等同于true。
您可以将main()放在每个函数的下方，从而不需要函数原型，因为它已经知道这些函数的存在。
您无需在return 0的末尾添加main()。 。这是一种特殊情况，因为编译器将自动插入return 0，因为到达此点始终表示成功终止。


评论

\ $ \ begingroup \ $
当心：iostreams与stdio在C ++程序员中引起争议。绝对避免使用scanf。
\ $ \ endgroup \ $
–匿名
2014年5月13日20:04

#3 楼

您可能要看的一件事是更有效的代码。您正在为要检查的每个号码运行一个循环。使用Eratosthenes筛网意味着您只需循环一次，向量的索引就会根据质数返回true或false：

#include <iostream>
#include <vector>
#include <cmath>

std::vector<bool> MakePrimes(int upperlimit)
{
    int bound = (int) floor(sqrt(upperlimit));
    upperlimit++;
    std::vector<bool> primes(upperlimit, true);

    primes[0] = false;
    primes[1] = false;
    //Since 2 is a special case if we do it separately we can optimize the rest since they will all be odd
    for(int i = 4; i < upperlimit; i += 2)
    {
        primes[i] = false;
    }
    //Since the only ones we need to look at are odd we can step by 2
    for (int i = 3; i  <= bound; i += 2)
    {
        if (primes[i]) 
        {
            //Since all the even multiples are already accounted for we start at the first one 
            //and skip to every other multiple
            for (int j = i*i; j < upperlimit; j += i * 2)
            {
                primes[j] = false;
            }
        }
    }
    return primes;
}
int main()
{
    int limit = 1000;
    std::vector<bool>checkPrime = MakePrimes(limit);
    int currentNum = 1;
    while (currentNum++ < limit)
    {
        if (checkPrime[currentNum]) 
            std::cout << currentNum << "\n";
    } 
    return 0;
}


#4 楼

此循环…

int currentNum;
currentNum = 2;
do {
    if (checkPrime(currentNum)) {
        printf("%d ", currentNum);
    }
    currentNum++;
} while (1 == 1);


…最好写成for循环：

for (int currentNum = 2; /* TODO: fix overflow */ ; currentNum++) {
    if (checkPrime(currentNum)) {
        printf("%d ", currentNum);
    }
}


出于可读性， checkPrime()应该重命名为isPrime()。它的参数应命名为number（小写）。

您正在使用蛮力试验分割算法。那行得通，但是当您想要包含许多质数的列表时，Eratosthenes的Sieve是一种效率更高的算法。