筛子的埃拉托色尼错误实现

Sieve of Eratosthenes buggy implementation

本文关键字：错误实现埃拉托更新时间：2023-10-16

我从c ++开始，我想实现埃拉托色尼的筛子。

#include <iostream>
#include <vector>
#include <cmath>
#include <ctime>
using namespace std;
vector<unsigned long long int> sieveOfEratosthenes(unsigned long long int min, unsigned long long int max) {
    vector<unsigned long long int> result;
    if (max <= 1) return result;
    if (min < 1) return result;

    if (max == 2) {
        result.push_back(max);
        return result;
    }
    if(min == 1) {
        min = 2; // skip 1 as it is not prime
    }
    bool primes[(max - min)+ 1];
    //Assume all number within range are prime and filter later
    for (unsigned long long int  i = min; i <= max ; i++) {
        primes[i] = true;
    }
    for (unsigned long long int  i = min; i < sqrt(max); i++ ) {
        if(primes[i]) {
            for (unsigned long long int  j = i; i*j <= max; j++) {
                primes[i*j] = false;
            }
        }
    }
    for (unsigned long long int  i = min; i <= max ; i++) {
        if (primes[i]) {
            result.push_back(i);
        }
    }
    return result;
}
void print(vector<unsigned long long int> result) {
    for ( auto &i : result ) {
        cout << i << std::endl;
    }
}
int main() {
    int start_s=clock();
    vector<unsigned long long int> result = sieveOfEratosthenes(1,1000000);
    int stop_s=clock();
    cout << "time: " << (stop_s-start_s)/double(CLOCKS_PER_SEC)*1000 << " ms" << endl;
    print(result);
    return 0;
}

当我运行它1-100时，我得到了正确的结果，但对于1-1000000我也得到了不正确的1000000。

编辑

我已经根据C++标准重构了代码，并提出了@badola和@Ben Voigt的建议.该代码也可以在Github上使用

#include <iostream>
#include <vector>
#include <cmath>
#include <ctime>
#include <fstream>
using namespace std;
using u_big = unsigned long long int;
vector<u_big> sieveOfEratosthenes(u_big min, u_big max) {
    vector<u_big> result;
    if (max <= 1 || min < 1 ) return result;
    if(min == 1) {
        min = 2; // skip 1 as it is not prime
    }
    if (max == 2) {
        result.push_back(max);
        return result;
    }

    /*
     * Declare a vector of boolean and ser all value to true
     * Consider all numbers to be prime at this point
     */
    vector<bool> primes(min + (max - min) + 1,true);
    auto sqrt_max = (u_big) sqrt(max);
    for (auto i = min; i < sqrt_max; i++ ) {
        if(primes.at(i)) {
            /*
             * Filter out non primes
             * Multiples of positive numbers cannot be primes
             */
            for (auto  j = i; i*j <= max; j++) {
                primes.at(i*j) = false;
            }
        }
    }
    for (auto  i = min; i <= max ; i++) {
        if (primes.at(i)) {
            result.push_back(i);
        }
    }
    return result;
}
void print(vector<u_big> result) {
    for ( auto const &i : result ) {
        cout << i << std::endl;
    }
}
void save(const string &filename,vector<u_big> result) {
    ofstream outFile(filename);
    for (const auto &p : result)  outFile << p << "n";
}
int main() {
    unsigned long long int min,max;
    cout << "Enter minimum : ";
    cin >> min;
    cout << "nEnter maximum : ";
    cin >> max;
    int start_s=clock();
    vector<u_big> result = sieveOfEratosthenes(min,max);
    int stop_s=clock();
    cout << "Runtime to generate primes : " << (stop_s-start_s)/double(CLOCKS_PER_SEC)*1000 << " ms" << endl;
    cout << "Number of primes : " <<  result.size() << endl;
    cout << "Primes saved to sieve.txt" << endl;
    save("primes.txt", result);
    //print(result);
    return 0;
}

问题，您正在访问数组分配内存之外的索引。
您需要初始化矢量以具有(最大 + 1(位置。

请更喜欢std::vector而不是 c 样式数组并使用at()运算符，并且每当程序运行超过最大分配索引时，它就会引发异常。

引发异常：terminating with uncaught exception of type std::out_of_range: vector

我重构了您的代码以适应 C++11，因为在评论中您提到了使用基于 C++11 的编译器。

#include <iostream>
#include <vector>
#include <cmath>
#include <ctime>
using u_big = unsigned long long int; // Whatever you want, please don't type it 
                                      // in multiple locations, too hard to update the code
                                      // Prefer using, from C++11
std::vector<u_big> sieveOfEratosthenes(u_big min, u_big max) {
    std::vector<u_big> result;
    if (max <= 1 || min < 1) return result;
    if (max == 2) {
        result.push_back(max);
        return result;
    }
    if(min == 1) min = 2; // skip 1 as it is not prime
    std::vector<bool> primes(max + 1, true); // set them all to true while creating vector
    // here was the real problem
    // you were actually accessing (max + 1) elements in your array
    // but were requesting for (max - min + 1) elements 
    for (auto i = min; i < sqrt(max); ++i) {
        if(primes.at(i)) { 
        // as suggested in the comments, use at()
        // you would have witnessed  your code throw exception
            for (auto j = i; i*j <= max; ++j) {
                primes.at(i * j) = false;
            }
        }
    }
    for (auto i = min; i <= max; ++i) {
        if (primes.at(i)) {
            result.push_back(i);
        }
    }
    return result;
}
template <typename Iterable>
void print(Iterable const & result) {
    for (auto const & i : result ) { // please use const &, if not modifying
        std::cout << i << ", ";
    }
    std::cout << std::endl;
}
int main() {
    auto start_s = clock();
    auto result = sieveOfEratosthenes(1,1000000);
    //auto result = sieveOfEratosthenes(1,100);
    auto stop_s = clock();
    std::cout << "time: " << (stop_s-start_s)/double(CLOCKS_PER_SEC)*1000 
    << " ms" << std::endl;
    print(result);
    return 0;
}