getters & setters are bad, design patterns etc.

[ne555]

> my getter checks whether the row is computed, and only if not, calls the function computing it.
TDA.

row[K].compute();

Row::compute(){
   if(this->already_computed)
      return;
   //computation algorithm
}

> int area () {return (x*y);} // getter
¿? that is not a getter

[Lachlan Easton]

¿? that is not a getter

I agree; I think we need to settle on a definition.

[TheIdeasMan]

Edit: I didn't see the last 3 posts while I was doing mine

Could I summarise by mentioning these points (not in any particular order of importance):

- When get / set functions are mentioned, people seem to immediately think of the trivial & naive versions - which really are bad.

- A trivial get function is much more benign than a trivial set function. But one should decide whether it is even necessary. That is only provide that interface if it is actually required externally to the class, as per the draw tangent line 2 circles problem - the values of centre point & radius are required at some point.

- Decide what data members should have some exposure outside the class. For example in an Arc class, we can calculate all the data members of the arc given 3 points on the arc. To do this we need to calc 2 line segments and their perpendicular bisectors intersect at the centre point.This is all done with a bunch of private functions & data. All that info shouldn't be exposed, but things like the centre point, start point, mid point, end point, radius, arc length, start angle, end angle should be. These things are Properties, but in C++ we provide that interface by using functions to retrieve them.

- Obviously prefer constructors to set values initially.

- Use a member function that gets input, rather than a trivial set function.

- Provide functions that get input and set several values, rather than a function for each one.

- If you have any kind of update function, then make sure you do checking & validation, so as not to invalidate the other data members. Think of move & rotate functions. For the Arc class we are quite clearly not going to have functions that independently alter any data member without checking or recalculating the other members if the operation is possible.

- Don't use get functions to perform output - provide a member output function instead.

- Part of what encapsulation means is to put all the functions to do with an object in the class. It seems to me that is what "The Law of Demeter" is about. For example you wouldn't have a BankAccount class with the functions Withdraw & Deposit external to any class (in main() say). They would probably be part of a Transaction class, which interacts with BankAccount classes.

- The "Tell, don't ask " idea seems to be the "USES A" relationship:

void Line::CalcTangentLine(const CCircle& Circle1, const CCircle& Circle2 ){}

I have used a void return type, but a return type that allowed some status error would be better. I am not sure whether that is better / easier than exceptions. There is a need to have a way of deciding which tangent line to draw, but the example was to show the "USES A" relationship. The circle objects still have getRadius & getCentrePoint as part of their interface.

To me, sending a value as a parameter is the exact same thing as a get function - only the name has changed:

inline double CCircle::sendRadius()const {return m_Radius;}
inline double CCircle::getRadius()const {return m_Radius;}

- Prefer to use Design Patterns to reduce coupling between classes.

- Upgrade data members to classes if that helps with future changing requirements, but don't get carried away with it. If future changing requirements aren't mentioned in the specification then don't do it.

- Upgrade data members to classes if that helps with making interfaces easy to use correctly & hard to use incorrectly. Scott Meyers has an example (Item 18) where he recommends having Day , Month & Year as classes in a Date object. His Month class has function like this:

class Month {
public:
static Month Jan() {return Month(1);}
//.....
private:

explicit Month (int m);
};

Date d( Month::Mar(), Day(30), Year(1995)  );

- Use templates to help future proof change in data type. For example if Circle uses templates to allow any integral or FP or boost multi-precision type in it's underlying data, then presumably one might do the same for anything that is a Property, as in:

Distance MyDistance = pMyCircle->getRadius;

That is, the Distance class uses templates in the same way as Circle does.

To me, this seems to be the solution to the problem (code breaks because type changes) posed in the article ne555 linked:

http://www.javaworld.com/javaworld/jw-09-2003/jw-0905-toolbox.html

The problem of a data member being extended in the future (Identity: Name, IDNum, Image) is solved by upgrading to a class.

Also, the solution proposed in the article was a get function with checking & validation.

So, is that a fair summary of the situation?

[ne555]

> Decide what data members should have some exposure outside the class.
> For example in an Arc class things like the centre point, start point, mid point,
> end point, radius, arc length, start angle, end angle should be (exposed)
¿all those wouldn't be data members, right?
I understand why do you may want to compute the `arc lenght' ¿but why outsiders need the `mid point'?
Define the responsibilities of your class

> Use a member function that gets input, rather than a trivial set function.
don't sure what you mean.
You may want to provide serialization to your class

> If you have any kind of update function, then make sure you do checking & validation,
> so as not to invalidate the other data members.
The operations should maintain the invariance.


> put all the functions to do with an object in the class.
> It seems to me that is what "The Law of Demeter" is about.
¿? Demeter says: "talk only to your immediate friends"
You interact with an ATM, as long as you care a BankAccount class may not exist


> The "Tell, don't ask " idea seems to be the "USES A" relationship:
TDA means `I don't know how to do this, but I know who can. If he needs any information in my power I'm happy to give it to him"
(don't know or is not my duty)

> void Line::CalcTangentLine(const CCircle& Circle1, const CCircle& Circle2 ){}
¿why is that a method of `Line' instead of `Circle'?

> To me, sending a value as a parameter is the exact same thing as a get function
¿where the `send value' comes from?
There is a big difference between `give me your wallet' and `here's my driver's license, officer'

> Use templates to help future proof change in data type.
> To me, this seems to be the solution to the problem (code breaks because type changes)
¿what about `we don't need that variable anymore'?

> Also, the solution proposed in the article was a get function with checking & validation.
¿could you please quote it?

[BHX]

¿? that is not a getter

I agree; I think we need to settle on a definition.

Well whether you call them setters or getters, accessors or mutators, or some other name they all do the same. Setters set the data for the object and getters get data from the object. The area function is getting the data from the class, multiplying it and then returning it. The only difference between the area function and sticking to the getWhatever() is that area removes some code. For example, if you want to be picky and stick to setters and getters only being setSome and getSome we can re-write the code like so:

#include <iostream>
using namespace std;

class CRectangle {
    int x, y;
  public:
    void set_values (int,int);  // setter
    int getX() { return x; } // getter
    int getY() { return y; } // getter
 //    int area () {return (x*y);}   // getter
};

void CRectangle::set_values (int a, int b) {
  x = a;
  y = b;
}

int area(int a, int b) { return (a* b); }

int main () {
  CRectangle rect;
  int locX, locY;
  rect.set_values (3,4);
  locX = rect.getX();
  locY = rect.getY();
  int rectangleArea = area(locX, locY);
  cout << "area: " << rectangleArea;
  return 0;
}

According to my text book from college accessors are the functions in classes to access the class object data. From that definition I consider area to be an accessor function. Not to mention it makes no sense to me that you can have mutators that set multiple variables, but any function that accesses the variables data and does something to them isn't an accessor.

[Lachlan Easton]

The area function is getting the data from the class, multiplying it and then returning it.

That's generally not what I think about when people mention getters. When I think get/set functions my mind jumps to classes that break their encapsulation with (practically) full access to it's private data (i.e. the naive approach).

[cire]

That's generally not what I think about when people mention getters. When I think get/set functions my mind jumps to classes that break their encapsulation with (practically) full access to it's private data (i.e. the naive approach).

Suppose we store the area and return it via the area() function. Later, we change it to calculate and return the value only when the area() function is called. At what point is it a "getter"? At what point is it not? At what point is it breaking encapsulation?

Again, getters and setters do not break encapsulation. They supply encapsulation and introduce the possibility of moving past the "naive" approach without impacting the interface other code uses to interact with the class instances, in contrast to the approach where encapsulation is eschewed and public access to all variables is given.

Yes, it's better to take the time to design your interface if it has a good probability of developing into a more complex one. No, that isn't true of all classes.

They're tools. And like any tools, one should know when to use them and not shy away because someone, somewhere said they do something that they don't actually do such as break encapsulation or indicate bad design. The former is never true and the latter is only true in some cases (and where it would be equally true of public variables.)

[TheIdeasMan]

@ne555

I understand why do you may want to compute the `arc lenght' ¿but why outsiders need the `mid point'?
Define the responsibilities of your class

On CAD (Computer Aided Design) systems, a facility called 'grips' is provided. When you mouse over a dwg object, the grips are highlighted. They are used to enable quick drawing, by allowing the user to 'snap' to various points (end, mid, perpendicular, tangent, parallel, cardinal points etc). They can also be used to modify an object by stretching it. In the case of an arc, one can drag around end or mid points and the whole thing recalculated & redrawn in real time.

> Use a member function that gets input, rather than a trivial set function.
don't sure what you mean.
You may want to provide serialization to your class

I mean a member function that uses cin to get input and assign it to member variables, rather than the cin being used in main() say, then the trivial set function being called with the value as an argument.

> If you have any kind of update function, then make sure you do checking & validation,
> so as not to invalidate the other data members.
The operations should maintain the invariance.

Well, yes. I mentioned the how one would not have a setCentrePoint that does straight assignment for an Arc object. The function needs to do the right thing.

> put all the functions to do with an object in the class.
> It seems to me that is what "The Law of Demeter" is about.
¿? Demeter says: "talk only to your immediate friends"
You interact with an ATM, as long as you care a BankAccount class may not exist

OK that makes sense.

> The "Tell, don't ask " idea seems to be the "USES A" relationship:
TDA means `I don't know how to do this, but I know who can. If he needs any information in my power I'm happy to give it to him"
(don't know or is not my duty)

Is that not the same thing? Wouldn't it be achieved by sending a reference to an object?

> void Line::CalcTangentLine(const CCircle& Circle1, const CCircle& Circle2 ){}
¿why is that a method of `Line' instead of `Circle'?

Because it is 'Line' that is being calculated (& drawn eventually). It is just that it "USES" the info from the circle objects. The 'info' is provided by the circle's interface because they are Properties.

> To me, sending a value as a parameter is the exact same thing as a get function
¿where the `send value' comes from?
There is a big difference between `give me your wallet' and `here's my driver's license, officer'

In the other thread (which in part prompted this one), you talked about sending 'Mass' as an argument. Also you proposed sending a reference to a PhysicsObject - presumably it has a getMass function as part of it's interface.

`here's my driver's license, officer'

is an example of a good interface, while

`give me your wallet'

is an example of a bad one.

> Use templates to help future proof change in data type.
> To me, this seems to be the solution to the problem (code breaks because type changes)
¿what about `we don't need that variable anymore'?

So templates is one part, using classes & Design Patterns are others. Even so it hard to see how to cope with this - taking part of an interface away would break any code that uses it. Apart from advertising that that part of the interface is deprecated what would you do?

> Also, the solution proposed in the article was a get function with checking & validation.
¿could you please quote it?

My mistake, I meant the function in the paper boy example:

public float getPayment(float bill) {
     if (myWallet != null) {
           if (myWallet.getTotalMoney() > bill) {
               theWallet.subtractMoney(payment);
               return payment;
           }
      }      
}

Notice that the Customer no longer has a 'getWallet()' method, but it does have a getPayment
method. Lets take a look at how the paperboy will use this:

// code from some method inside the Paperboy class...
payment = 2.00; // “I want my two dollars!”

paidAmount = myCustomer.getPayment(payment);
   if (paidAmount == payment) {
           // say thank you and give customer a receipt
   } 
   else {
           // come back later and get my money
    }

@everyone

I forgot to mention the obvious point that having a public get function and a public set function which is straight assignment is the same having a public variable. This is what people tend to think of , and we all agree that this is really bad.

cire wrote:

Suppose we store the area and return it via the area() function.

That's a very good point - especially when the calculation is long & expensive. I have just implemented this:

// http://www.icsm.gov.au/gda/gdatm/gdav2.3.pdf
// Vicenty's Inverse Formulae
// Chapter 4, page 15 - page 20 in pdf

I would be nuts not to store the answers in a member variable, so that it could be output to a file, or used by another class. Especially when I have 50,000 Lat /Longs to convert to MGA coordinates (Almost as tricky as Vicenty Inverse).

BTW - has anyone have any answers for my other post about using boost? It is real easy to write my own code - but I wanted to have a go with the library.

Any way I hope we are all having fun !

[LB]

cire wrote:

with getters you could change the internal representation to do that without affecting the existing interface. Using const reference members, you cannot.

The const alias members can easily be replaced with mimics or proxies, as I mentioned earlier.

@what-is-a-getter: I agree that a getter is when you directly return a variable that is part of the class' state.

cire wrote:

Suppose we store the area and return it via the area() function.

That is a cached variable and would be mutable, I don't consider it part of the object's state.

[ne555]

@BHXSpecter: all member functions should use or modify the state of the objects.
It wouldn't make sense for them to be member functions otherwise.
So according to your interpretation, all member functions are accessor, which makes the classification useless.

> Again, getters and setters do not break encapsulation
Look at BHXSpecter code and say that again.


> The "Tell, don't ask " idea seems to be the "USES A" relationship:
> Is that not the same thing? Wouldn't it be achieved by sending a reference to an object?
> void Line::CalcTangentLine(const CCircle& Circle1, const CCircle& Circle2 ){}
> Because it is 'Line' that is being calculated (& drawn eventually).
> It is just that it "USES" the info from the circle objects.
Or easier, the circles know their centre point and radius. They are able to calculate the tangent line.

//this function has an awful name
void Line::CalcTangentLine(const CCircle& Circle1, const CCircle& Circle2 ){
   *this = circle1.CalcTangentLine(Circle2);
}

void CCircle::CalcTangentLine(const CCircle& Circle2) const{
   //no need for getters here, you've got direct access to your private members
}

> In the other thread (which in part prompted this one),
> you talked about sending 'Mass' as an argument.
> Also you proposed sending a reference to a PhysicsObject
> - presumably it has a getMass function as part of it's interface.
In the first case, it does not.

Object::some_action(params){
	OPhysics.calculation_that_uses_the_mass( this->mass );
}

> Also, the solution proposed in the article was a get function with checking & validation.
No, as you don't have a `getWallet()' you don't need a wallet anymore.
It is not `checking & validation' but obtain money logic.

[BHX]

> Again, getters and setters do not break encapsulation
Look at BHXSpecter code and say that again.

Encapsulation Definition: In Object Oriented Programming, encapsulation is an attribute of object design. It means that all of the object's data is contained and hidden in the object and access to it restricted to members of that class.

How did I break encapsulation exactly? The data members are still restricted to the class in my code. I simply copied their data to another variable which is a copy of the data and not actually touching the data held in the class object.

[LB]

When the internal class implementation changes (e.g. you change from age being in years to age being determined as the time difference between the date of birth and the current date), the public interface should not change.

[TheIdeasMan]

@ne555

Or easier, the circles know their centre point and radius. They are able to calculate the tangent line.

OK, I finally think I can see what you are saying (HOORAY!!), but more like the physics example. I think the extra level of indirection needed in the object's SendData function, and the fact that there is more than one object involved is the part I didn't get until now. See what you think of this:

// Receives data from the circles
// does calculations
// creates a new line & puts it in a contaner
class TangentLine {
private:

double Circ1Radius;
CPoint2D Circ1Centre;
 
double Circ2Radius;
CPoint2D Circ1Centre;

CPoint2D LineBegPt;
CPoint2D LineEndPt;

//other variables needed to do the calculation 
void Calc()

public:

TangentLine();

// these functions are called by the circle objects
void Circle1Data(const CPoint2D Centre, const double Radius ) : /*initialiser list */ {}
void Circle2Data(const CPoint2D Centre, const double Radius ) : /*initialiser list */ {}

};

void TangentLine::Calc() {
    // do the calcs using the now private data
    // create a new Line on the heap using the calc'd data in it's constructor
    // push it into whatever container it belongs to
}

The Circle class has void SendDataObj1 & void SendDataObj2 functions which call TangentLine::Circle1Data or TangentLine::Circle2Data respectively, using their private data which they have direct access to.

So we need another class or function somewhere that does this:

Circle1.SendDataObj1;
Circle2.SendDataObj2;

I guess this could go into a NewDwgEntity class which would also be responsible for calculating other kinds of drawing objects.

Ok, here is the crux of the matter: The Circle objects no longer need a public interface, but we have managed to send their data to another process, while keeping it away from everything else. The details of the Circles are kept away from the Line object.

The cost is a bit more complexity and the use of a Design Pattern such as Command say would allow the expansion of it 's use to a variety different situations.

To me this is solved now - yay!! Thanks for everyone's contributions, thoughts, time & effort. Especially ne555 for putting up with my stubbornness. I will leave the post open for a couple of days - who knows there might be another 5 pages worth on it's way!! Have a beer for me this weekend, I will have several for you guys :)

ThingsLearnt += Points(100);

[ne555]

¿wtf is that?

Line Circle::CalcTangentLine(const Point &P) const;
Line Circle::CalcTangentLine(const Circle &C) const;

[Oria]

Just want to test ne555 issue, as per thread:
http://www.cplusplus.com/forum/lounge/101716/
please ignore this post.

[BHX]

So can someone tell me how my code breaks encapsulation? As far as I can see it doesn't as every member is still restricted to the class and I'm only manipulating copies of the data in the class object.

[3qX21hU5]

Well your class is designed wrong.

class CRectangle {
    int x, y;
  public:
    void set_values (int,int);  // setter
    int getX() { return x; } // getter
    int getY() { return y; } // getter
 //    int area () {return (x*y);}   // getter
};

void CRectangle::set_values (int a, int b) {
  x = a;
  y = b;
}

int area(int a, int b) { return (a* b); }

What is the point of the area function? Why should the user have to store the results of the getter functions getX and getY and then pass them stored results to the area function? This is making the user do more work, and is totally not needed.

I would also argue what is the difference in using your getters and setters, and just having the variables x and y public? There isn't much really, the user can still enter bad input, the user can still make changes to the underlying variables that can break the implementation.

The way you class is designed is just like having a class like this

class Rectangle
{
    public:
        int x;
        int y;
};

int area(int a, int b) { return a * b; }

Sure you are using member functions to set the underlying data members but it is essentially the same.

[TheIdeasMan]

@ne555

In your latest example C is a reference to the second Circle object - how do you access it's private data? Via it's interface? Which looks like what?

I thought I had a way of the Circles not needing an interface at all (apart form sending data to the class that does the calcs ), but now I am confused as to why you think that is not the case.

[BHX]

@Zereo
What?! Has everyone lost their senses in this thread? The whole point of encapsulation is to make it so the user doesn't have direct access to the members (keep them restricted to the class). My method keeps encapsulation while your methods blow it completely out of the water. I did the extra work on purpose to show that area is manipulating copies and not the actual members of the class, I could have just as easily did:

cout << "area: " << area(rect.getX(), rect.getY()) << endl;

Still keeps encapsulation in place because the only way to change rect's data is through set_values(int, int) where as the user could accidentally change a public member by accident and not catch the error for a long time depending on how large the code is. This is why I'm am utterly blown away when I see programmers saying setters and getters are bad, because I feel that making class members public is bad. If you want class members public use a struct, but the whole point of classes is to promote encapsulation, which is impossible if you make them public after making them.

I did forget to make my getters constants, but that is a minor fix.

[cire]

Zereo wrote:

I would also argue what is the difference in using your getters and setters, and just having the variables x and y public? There isn't much really, the user can still enter bad input, the user can still make changes to the underlying variables that can break the implementation.

It's fine if you don't believe the benefits of using getters and setters outweighs the work to use them. It's fine if you prefer to greatly increase the complexity of your code by using mimics and proxies to avoid a simpler, less error prone solution because it makes you feel clever. It's even fine if you want to play semantic games so you can say a getter is not a getter..

But, can we at least pay attention to what's been said so far?