patterncppMinor
Find all the primes less or equal than N
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
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
-
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
Return
Naming
The following is confusing:
You have both multiple
Loops
Do you have a particular reason why the following is a
A
Now we're talking about that loop anyway, only use one letter variables like
There's no rule against trailing underscores (my personal preference is not to use them), but in this case the difference between
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.