views:

71

answers:

5

I have these classes:

class Base
{
...
private:
std::vector<X> v;
};

class Derived : public Base
{
Derived(X*, int n);
};

where the constructor of Derived is passed an array of item Xs, which I need to insert into my vector v in the Base class. (X is a smart pointer)

Currently I see two ways to do this:

  1. Create a function in Base: InsertItem(X*) that will insert an item into the vector (1 by 1)
  2. Create a vector in Derived that contains the full list, then get it into Base by moving the entire vector.

I dont see any advantages to #2, but was wondering if #1 was a good solution, or if there are better ways to do this.

Thanks!

+4  A: 

There is a third option if you can modify Base. You could make the vector protected which will allow base classes full access:

class Base
{
...
protected:
    std::vector<X> v;
};

Your base class can now directly operate on v. If you need to expose much of the vector functionality to Derived, this is the easiest way.

If not, I would just add the one function (i.e. option #1) and would make it protected:

class Base
{
...

protected:
    void push_back(const X &x)
    {
        v.push_back(x);
    }

private:
    std::vector<X> v;
};
R Samuel Klatchko
Changing from `private` to `protected` is so obvious, I would just assume there are reasons it can't be done. And isn't your other suggestion just a repeat of item 1. in the question?
Mark Ransom
Yes, my second suggestion is just a repeat of #1 - the idea was if he didn't like protected, my advice would be to prefer #1 over #2.
R Samuel Klatchko
I've currently got it implemented as #1. Thanks for reminding me to use a const reference.
Will
I don't like the first option, `push_back` is much better from an encapsulation point of view.
Matthieu M.
+1  A: 

If you can, I would create a protected constructor in Base that accepts the X* argument just like the Derived constructor, and then in Derived's constructor initialization list, pass the argument to Base's protected constructor and have it do the actual insertion.

rmeador
A: 

You should set the member variable to be protected so that classes that inherit Base can access it. Alternatively keep the member variable private but add a protected accessor method to allow classes that inherit base to add to the vector etc

Chris
No, that's a breach of encapsulation. Better to provide full fledged methods.
Matthieu M.
+3  A: 

Another option is to create a (public or protected) constructor in base that will take the parameters and populate the vector directly. No intermediate vectors need be created or passed around.

class Base
{
 . . .
protected:
    Base(X* x,int n);
 . . .
};

class Derived : public Base
{
    Derived(X* xes, int n):Base(xes,n){};
};
zdan
+1  A: 

If you want the container in the base class to remain private, you might consider adding a public (or protected) function in Base to allow the derived class to add a range. This way, the Base container type can be encapsulated (Derived doesn't need to know it's a vector), and Derived can still pass the whole thing down in one shot:

class Base
{
...
private:
    std::vector<X> v;

protected
    template <class iterator_t>
    void assign( iterator_t first, iterator_t last) {
        v.assign( first, last);
    }
};

class Derived : public Base
{
Derived(X* p, int n) {
    Base::assign( p, p+n);
}
};

It's not much of an encapsulation, but the container could be changed with a changing the derived class, and a derived class could easily be written that takes data in a form different than an array, if desired. And as zdan suggested this functionality could be part of public or protected constructor for Base if that makes more sense (which is likely).

The idea of using iterator ranges to represent the content or subset of a container is widespread in the STL.

Michael Burr