Vectors Classes Private/Public

Question

In C++ is always better to keep data of a class as private members.
If a class has a vector as member is better to put it as a private or public member?

If I have a vector as private member I cannot easily access to the member function of the vector. So I have to design the class with a method for every function I need to access the vector methods?

Example given:

class MyClass{
private:
     std::vector<int> _myints;
public:
     get_SizeMyints(){return _myints.size();}
     add_IntToMyints(int x){_myints.push_back(x));
};

or is better to keep the vector public and call MyClass._myints.push_back(x)?

---------------------edit--------------

and just for clarity for what is needed this question:

snake.h:

enum directions{UP, DOWN, RIGHT, LEFT, IN, OUT, FW, RW };


class Snake
{
private:
    enum directions head_dir;
    int cubes_taken;
    float score;
    struct_color snake_color;
    V4 head_pos;


public:

    std::vector<Polygon4> p_list; //the public vector which should be private...

    Snake();
    V4 get_head_pos();
    Polygon4 create_cube(V4 point);
    void initialize_snake();
    void move(directions);

    void set_head_dir(directions dir);
    directions get_head_dir();
    void sum_cubes_taken(int x);
    int get_cube_taken();

    void sum_score(float x);
    float get_score();

    void set_snake_color();



};

so now I know how to change the code.

btw... a question, if I need to copy the vector in an other class like this: GlBox.p_list = Snake.p_list (works if are private) what will be an efficent method if they where private?
Running a for cycle to copy the the elements and pusshing back them in the GLBox.p_list seems a bit inefficent to me (but may be just an impression) :(

Answer 1

If it doesn't matter if someone comes along and empties the vector or rearranges all it's elements, then make it public. If it matters, then yes, you should make it protected/private, and make public wrappers like you have. [Edit] Since you say "it's a snake", that means it'd be bad if someone came and removed or replaced bits. Ergo, you should make it protected or private. [/Edit]

You can simplify a lot of them:

MyClass {
private:
     std::vector<int> _myints;
public:
     const std::vector<int>& get_ints() const {return _myints;}
     add_IntToMyints(int x){_myints.push_back(x));
};

That get_ints() function will allow someone to look at the vector all they want, but won't let them change anything. However, better practice is to encapsulate the vector entirely. This will allow you to replace the vector with a deque or list or something else later on. You can get the size with std::distance(myobj.ints_begin(), myobj.ints_end());

MyClass {
private:
     std::vector<int> _myints;
public:
     typedef std::vector<int>::const_iterator const_iterator;
     const_iterator ints_begin() const {return _myints.begin();}
     const_iterator ints_end() const {return _myints.end();}
     add_IntToMyints(int x){_myints.push_back(x));
};

Answer 2

For good encapsulation, you should keep your vector private.

Answer 3

Your question is not very concrete, so here's an answer in the same spirit:

Generally, your classes should be designed to express a particular concept and functionality. They should not just hand through another member class. If you find yourself replicating all the interface functions of a member object, something is wrong.

Maybe sometimes you really just need a collection of other things. In that case, consider a plain old aggregate, or even a tuple. But if you're designing a proper class, make the interface meaningful to the task at hand, and hide the implementation. So the main question here is, why do you need to expose the vector itself? What is its role in the class? What does its emptiness signify in terms of the semantics of your class?

Find the appropriate idioms and ideas to design a minimal, modular interface for your class, and the question might just go away by itself.

(One more idea: If for example you have some range-based needs, consider exposing a template member function accepting a pair of iterators. That way you leverage the power of generic algorithms without depending on the choice of container.)

Answer 4

Normally, good coding practice is to keep your data members private or protected, and provide whatever public methods will be needed to access them. Not all the methods of (in this case) vector, just what will be useful for your application.

Answer 5

That depends on your class's purpose. If you're trying simply trying to wrap the vector and want to use it as a vector you could make an argument for making the vector public.

Generally speaking I would suggest making it private and providing an appropriate interface to manipulate the container. Additionally this lets you change the container under the hood if a different container would ever be more appropriate (as long as you don't tie your public interface to the container type).

Further as an aside, avoid names that begin with underscores as there are some such identifiers reserved for the implementation and it's safer to just avoid all of them rather than trying to remember the rules in all cases.

Answer 6

A point to realize is that making the std::vector private is only half of the story when it comes to good encapsulation. For example, if you have:

class MyClass {
    public:
        // Constructors, other member functions, etc.

        int getIntAt(int index) const;

    private:
        std::vector<int> myInts_;
};

...then arguably, this is no better than just making myInts_ public. Either way, clients will write code using MyClass which is dependent on the fact that the underlying representation requires the use of a std::vector. This means that in the future, if you decide that a more efficient implementation would utilize a std::list instead:

class MyClass {
    public:
        // Constructors, other member functions, etc.

        int getIntAt(int index) const; // whoops!

    private:
        std::list<int> myInts_;
};

...now you have a problem. Since you can't access into a std::list by index, you would either have to get rid of getIntAt, or implement getIntAt using a loop. Neither option is good; in the first case, you now have clients with code that doesn't compile. In the second case, you now have clients with code that just silently became less efficient.

This is the danger of exposing any public member functions which are specific to your choice of implementation. It's important to keep flexibility/future maintenance in mind when designing your class interface. There are a number of ways you could do this with your particular example; see Mooing Duck's answer for one such interface that exposes iterators.

Or, if you would like to maximize code readability, you could design the interface around what MyClass logically represents; in your case, a snake:

class MyClass {
    public:
        // Constructors, etc.

        void addToHead(int value);
        void addToTail(int value);
        void removeFromHead();
        void removeFromTail();

    private:
        // implementation details which the client shouldn't care about
};

This offers an abstraction of a snake object in your program, and the simplified interface gives you the flexibility to choose whatever implementation suits it best. And if the situation arises, you can always change that implementation without breaking client code.

Answer 7

Theoretically in Object Oriented Programming any attributes should be private and gain access to them via public methods such as Get() and Set(). I think you question is not complete, but what I understand from what you're trying to achieve you need to inherit from std::vector and extend its functionality, to both satisfy your fast access needs and not messing around with encapsulation. (Consider reading on "Inheritance" first from any C++ book, or other OO language)

Having said that, your code might look as following:

class MyClass : public std::vector<int>
{
     //whatever else you need goes here
}

int main(void)
{
     MyClass var;
     var.push_back(3);
     int size = var.size(); // size will be 1
}

Hope this answered your question