class privacy

[prometheus]

can someone explain why you would use 'private' for members of a class and then have public accesor functions? It is much more work to do and I can't figure out why you wouldn't just make the class members public in the first place.

[oulyt]

by member do you mean member functions or data members.

data members you make private so they can't be directly edited and so people can't mess with the variables. member functions you almost never make private unless it's some special case. at least from what i know, which to tell you the truth is not much at all.

[C-Man]

well many reasons , one being you might want to change the internal representation of data without wanting to affect the code that uses it
you might want to monitor writes/reads and do something when it happens

[myork]

It is to protect the implementation.
The public functions are your interface to the object. You can change the internal (private) representation without affecting anything that uses the code because the external (public) representation does not change.

Code:

   // First attempt at your code.
    class Point
    {
        public: Point(int x,int y):  m_x(x),m_y(y) {}
            int getX()  {return x;}
            int getY()  {return y;}

           int getAngle() { return someAngleMaths(x,y);}
           int getDist()    { return someDistMaths(x,y);}
        private:
            int m_x;
            int m_y;
    };

This seems like a reasonable implementation for a point and you write a lot of code that uses it.
Now you go along for a while with no problems. You then realize that your code is not fast enough. You trace the problem back to the point class as a lot of work goes into getting the polar coordinates all the time.

Since you protected the internal representation from an external user you can change it to make the class more efficient without affecting any of the users.

Code:

   class Point
    {
        public: Point(int x,int y):  m_a(someAngleMaths(x,y)),m_d(someDistMaths(x,y)) {}
            int getX()  {return convertPolarToX(m_a,m_d);}
            int getY()  {return convertPolarToY(m_a,m_d);}

           int getAngle() { return m_a;}
           int getDist()    { return m_d;}
        private:
            int m_a;
            int m_d;
    };

Daaa. Daa.
More efficient code. Nobody will even know you changed anything.

[prometheus]

ah that makes sense, so in the long run for a complex program you are saving work. thanks all.

[myork]

Even in small programs it will save work.
You may think you know how somthing should work, but once you start digging into the details your assumptions are nearly always blown away. By restricting access to member variables you protect yourself from changing the whole program just to get around some minor incorrect assumption. And the amount of work is usually negligible.


Also note: Don't add getter/setter method just because (unlike Java that is considered bad practice in C++ (In Java it is sort of useful because it used by frameworks and reflection to build autamated tools)).

But when you do add them they should look like this:

Code:

class Value
{
    private:
        T    m_value;
    public:
        T const&  getValue() const;
        T&        getValue();
};

Notice the lack of setValue(). We don't want that in C++. The object should already be part of the class. So the getValue() returns a reference to the internal value. You can modify it via the reference. If you are using a setValue() then that is for adding a pointer (make it smart pointer) to an object.

Code:

class ValueAsP
{
    private:
        boost::shared_ptr<T>   m_value;
    private:
        T const&  getValue() const;  // de-reference the pointer before returning or throw an exception for NULL.
        T&        getValue();
        void      setValue(std::auto_ptr<T> newValue);      // Allows usr to pass RAW pointers in a way that indicates that you will take ownership
        void      setValue(boost::shared_ptr<T> newValue);
};

Note. All rules are there as guidance. Modify as required for your situation.

[charlie]

Also note: Don't add getter/setter method just because (unlike Java that is considered bad practice in C++ (In Java it is sort of useful because it used by frameworks and reflection to build autamated tools)).

But when you do add them they should look like this:

Code:

class Value
{
    private:
        T    m_value;
    public:
        T const&  getValue() const;
        T&        getValue();
};

Notice the lack of setValue(). We don't want that in C++. The object should already be part of the class. So the getValue() returns a reference to the internal value. You can modify it via the reference. If you are using a setValue() then that is for adding a pointer (make it smart pointer) to an object.

Code:

class ValueAsP
{
    private:
        boost::shared_ptr<T>   m_value;
    private:
        T const&  getValue() const;  // de-reference the pointer before returning or throw an exception for NULL.
        T&        getValue();
        void      setValue(std::auto_ptr<T> newValue);      // Allows usr to pass RAW pointers in a way that indicates that you will take ownership
        void      setValue(boost::shared_ptr<T> newValue);
};

Note. All rules are there as guidance. Modify as required for your situation.

I disagree with this advice (at least for the moment when I can't see why you'd do it).

If you return a non-const reference to the internal data, then you are making the exact same mistake as making it public in the first place. What if you want to switch to storing the angle and distance instead of the x and y coordinates now? What if you want to add error checking for when the user sets a new point? You can't do that when returning a non-const reference.

[myork]

What if you want to add error checking for when the user sets a new point? You can't do that when returning a non-const reference.

Yep that's worth mentioning.
Yep ignore my stuff use a setValue() so that you don't expose the internal representation.