Your PrimeList
doesn't follow object-oriented design principles; it's just a thin wrapper for a std::vector
that doesn't do much.
A goal of object-oriented design is to create objects as "smart" data. Your PrimeList
class should know how to maintain itself in a self-consistent state. PrimeList
is supposed to know all about how to find prime numbers. Users of the class should be able to ask it what the nth prime number is. Instead, you have made your main()
function do all the hard work.
Think about how users of your class would want to call your code. For example, this would be a nice interface:
int main(int argc, char *argv[]) {
PrimeList primes;
// http://en.wikipedia.org/wiki/List_of_prime_numbers#The_first_500_prime_numbers
std::cout << primes[0] << " should be 2" << std::endl;
std::cout << primes[499] << " should be 3571" << std::endl;
return 0;
}
Now, let's design the class to make that happen. Here's a start:
class PrimeList {
public:
PrimeList();
// Returns the nth prime number
// (primelist[0] is 2, primelist[1] is 3, etc.)
unsigned long operator[](size_t nth);
};
To make that work, though, PrimeList
needs to keep some state internally. I would use a vector for running the Sieve of Eratosthenes. For convenience and efficiency, I would keep another vector of just the primes that have been discovered using that sieve. I also need to have some code for creating and maintaining that sieve. That code and the two vectors are part of the internal workings of PrimeList
, and are none of the caller's business. Therefore, they should be declared as private
members of the PrimeList
class:
class PrimeList {
public:
PrimeList();
// Returns the nth prime number
// (primelist[0] is 2, primelist[1] is 3, etc.)
unsigned long operator[](size_t nth);
private:
std::vector<bool> sieve; // sieve[i] is true if i is prime
std::vector<unsigned long> knownPrimes;
void findMorePrimes();
};
Then, it's just a matter of filling in the code. =) Here is a possible solution.
PrimeList::PrimeList() {
sieve.push_back(false); // 0 is not prime
sieve.push_back(false); // 1 is not prime
knownPrimes.push_back(2);
}
unsigned long PrimeList::operator[](size_t nth) {
while (nth >= knownPrimes.size()) {
findMorePrimes();
}
return knownPrimes[nth];
}
void PrimeList::findMorePrimes() {
// Initially, sieve is a vector where every element is assumed
// to be prime (sieve[i] = true).
// Then, we will set sieve[i] = false for all i where i is non-
// prime. Special cases for non-primes sieve[0] = false and
// sieve[1] = false were already established in the constructor.
// Enlarge the sieve by some arbitrary amount
std::vector<bool>::size_type prevSieveSize = sieve.size();
sieve.resize(4 * prevSieveSize, /* is_prime= */ true);
// Strike out the composite numbers from the enlarged sieve
for (std::vector<unsigned long>::iterator p = knownPrimes.begin();
p != knownPrimes.end();
++p) {
for (std::vector<bool>::size_type multiple = (prevSieveSize / (*p)) * (*p);
multiple < sieve.size();
multiple += (*p)) {
sieve[multiple] = false;
}
}
// Harvest the newly discovered prime numbers
for (std::vector<bool>::size_type i = prevSieveSize; i < sieve.size(); ++i) {
if (sieve[i]) {
knownPrimes.push_back(i);
}
}
}
Note that in order to maintain the illusion of an unbounded list of primes, findMorePrimes()
is more complicated than an implementation of the Sieve of a predetermined size. That additional bookkeeping is the price to be paid for the desire to present a pretty interface to the outside world. It's also illustrates the power of object-oriented programming to manage complexity, as you can see in the simplicity of the main()
example.
sqrt
should bestd::sqrt
. Unfortunately, some compilers don't warn about this, and then we're in for a nasty surprise when the code suddenly refuses to compile on another platform. – Matt Jul 9 '13 at 22:38std::
but still called the function. I have no idea why some compilers don't catch this. – Jamal♦ Jul 9 '13 at 23:22std::abs
(will work ondouble
and others) with C'sabs
(will only work withint
). When a "lenient" (but actually unhelpful) compiler doesn't warn about a misuse like this (which happens all to often :[), the same source code may produce completely wrong results on another platform :-( Really wish compilers (looking at MSVC here in particular) were more strict about this. – Matt Jul 10 '13 at 0:39