Skip to main content
Code Review

Return to Revisions

2 of 2
replaced http://codereview.stackexchange.com/ with https://codereview.stackexchange.com/

I think the object-oriented design could be improved.

  • PrimeGenerator is a tool to generate a linked list. Once you return a pointer to it, who knows what the caller will do with its contents? The caller could alter the list in a way that breaks subsequent calls to .addPrime().
  • Primes are a notionally infinite list, so your class should try to create the illusion of an infinite list. Requiring the user to call .addPrime() is a chore that breaks that illusion.

It would be better if PrimeList were a self-managing read-only list of prime numbers, like this:

class PrimeList {
 public:
 PrimeList();
 // Returns the nth prime number
 // (primelist[0] is 2, primelist[1] is 3, etc.)
 unsigned long long operator[](size_t nth);
 private:
 std::vector<unsigned long long> knownPrimes;
 void findMorePrimes();
};

Then your implementation of operator[] could call findMorePrimes() as needed behind the scenes.

The notion of an infinite list also frees you from the expectation that you should only add one prime number at a time to the list. Then, you would have the flexibility to find batches of prime numbers at a time, with likely improvements in performance.

Answer self-plagiarized from here.

200_success
  • 145.6k
  • 22
  • 190
  • 479
default

AltStyle によって変換されたページ (->オリジナル) /