HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

Find all the primes less or equal than N

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
theprimesallequalthanlessfind

Problem

This is not a novel interview problem. But I want to address it with the following points:

-
Implement the algorithm with Object-Oriented Design principles in mind.

-
Optimize the solution, if there are many continuous inputs.

-
Write simple unit tests (without using unit testing framework like gtest, boost.test)

Basically, I followed the idea of Sieve of Eratosthenes algorithm. To optimize the solution if there are many continuous inputs, I maintain a vector which stores all the primes we have computed so far. If the new n is smaller than the previous maximal n, we can directly get the primes from "cache" without re-computing the primes again.

I'm specifically looking for suggestions on the algorithm itself, OOD or better unit tests.

```
#include
#include
#include
#include
#include
using namespace std;

class PrimeFinder {
friend class PrimeFinderTest;
public:
PrimeFinder(int n = 2) {
is_prime_.push_back(false);
is_prime_.push_back(false);
is_prime_.push_back(true);
primes_.push_back(2);
n_ = n;
}

vector get_primes(int n) {
if(n is_prime_;
vector primes_;
int n_;

vector get_primes_subset(int n) {
auto iter = primes_.begin();
while(iter != primes_.end()) {
if(*iter (primes_.begin(), iter);
}

void extend_primes(int n) {
is_prime_.reserve(n + 1);
is_prime_.insert(is_prime_.end(), n - n_, true);

int bound = sqrt(n);
for(int i = 2; i n_) {
is_prime_[j] = false;
}
j += i;
}
}
}

for(int i = n_ + 1; i
string vec2str(const vector& vec) {
string str = "";
for(auto n : vec) {
str += to_string(n) + " ";
}
return str;
}

class PrimeFinderTest {
public:
void test_get_primes_subset(vector origin, int n) {
PrimeFinder test;
test.primes_ = origin;
vector su

Solution

Namespaces

using namespace std; is considered bad practice. Short code is not a requirement in C++, clear code is preferred.

Return

return 0; is a legacy from C. In C++, it's no longer required to write this manually at the end of main. The compiler will take care of returning 'normal' if no errors where thrown or other returns (like -1) are encountered.

Naming

The following is confusing:

PrimeFinder test;
PrimeFinderTest test;


You have both multiple PrimeFinder around which you named test as a PrimeFinderTest named test. In all cases the variable name is not descriptive and together it's confusing.

Loops

Do you have a particular reason why the following is a while loop?

while(j  n_) {
                    is_prime_[j] = false;
                }
                j += i;
            }


A for loop would be more readable here.

Now we're talking about that loop anyway, only use one letter variables like i and j when iterating (if at all, those extra bytes for full names won't bite you). Here, you're using both n and n_ without documenting what they are. That's a maintainability trap.

There's no rule against trailing underscores (my personal preference is not to use them), but in this case the difference between n and n_ is not distinctive enough. n_ = n; is also a good indication your naming could be improved.

Code Snippets

PrimeFinder test;
PrimeFinderTest test;
while(j <= n) {
                if(j > n_) {
                    is_prime_[j] = false;
                }
                j += i;
            }

Context

StackExchange Code Review Q#101657, answer score: 6

Revisions (0)

No revisions yet.