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

Object-oriented design of list of primes

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

Problem

Is there a better way to implement this class, or does it not constitute implementation in the first place? This is one of my first few tries at OOP. Any suggestions will be much appreciated.

#include 
#include 
#include 

class PrimeList
{
    public:
        std::vector vPrime;
        void initVector(int n) 
        {
            for (int i=2; i > n;

    // Create a vector filled with every integer from 2 to n.
    primes.initVector(n);

    // Begin the sieve
    for (int a=0; a < sqrt(n); ++a) {
        for (int b=a+1; b < primes.vPrime.size(); ++b) {
            if (primes.vPrime[b] % primes.vPrime[a] == 0) {
                primes.vPrime.erase(primes.vPrime.begin() + b);
            }       
        }
    }

    // Print the list of primes from 2 to n.        
    for (int i=0; i < primes.vPrime.size(); ++i) 
        std::cout << primes.vPrime[i] << std::endl;
    return 0;   
}

Solution

-
As a general rule for classes: data members (variables), such as vPrime, should be private. They can either be declared under private or outside of public (a class is private by default).

-
For a simple use of classes here, you don't need a mutator. Just declare your object with an argument:

PrimeList primes(n);


Since this would make n (or let's just make it number) a data member, modify your constructor as such and initialize the object like this:

// 'n' is the parameter and 'number' is the class's data member
PrimeList::PrimeList(int n) : number(n) {initVector();}


As you can see, the constructor now calls initVector(). With number now as a data member, initVector() no longer needs an argument. Since this function is not part of the interface, make it private.

-
The display for-loop in main() can also be put into a class method:

void display() const; // header declaration

// .cpp implementation

void PrimeList::display() const
{
    // your object is no longer referenced inside the class
    for (int i = 0; i < vPrime.size(); ++i)
    {
        std::cout << vPrime[i] << std::endl;
    }
}


Then simple call it in main() with your object:

primes.display();


Note the use of const fin this function. If you're not changing any data members, create your functions as such. If you accidentally violate this, your compiler will stop you. This is to ensure that your data members are not unintentionally modified, depending on the functions's purpose.

-
Your sieve calculations can also be put into a class method. Get the idea now? Again, it should be private. It will NOT be const since you're changing the vector inside the class. You can call it in initVector() since they're both non-const (you cannot call const functions in non-const functions and vice-versa).

Code Snippets

PrimeList primes(n);
// 'n' is the parameter and 'number' is the class's data member
PrimeList::PrimeList(int n) : number(n) {initVector();}
void display() const; // header declaration

// .cpp implementation

void PrimeList::display() const
{
    // your object is no longer referenced inside the class
    for (int i = 0; i < vPrime.size(); ++i)
    {
        std::cout << vPrime[i] << std::endl;
    }
}
primes.display();

Context

StackExchange Code Review Q#28306, answer score: 3

Revisions (0)

No revisions yet.