getters & setters are bad, design patterns etc.

[TheIdeasMan]

@cire

Just wondering whether you have any thoughts on the code I posted above?

[3qX21hU5]

@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.

I would really appreciate if you read what I posted before, saying I advocated you use public members instead.

I never once said that and actually agree that getters and setters should be used when they are needed and the right way (Which I have been saying throughout this entire thread...).

Yes the only way in your case to change rec's data members is through the set_values() function. But what I was trying to point out is that if the user can easily do myRec.y = -100; he can also easily do myRec.set_values(-100, 500);. Meaning that the user can still change the data without intending to, and can still enter bad input without intending to.

Yes having manipulating the data through a member function is always better then having direct access to the data. But you must consider other things also.

@cire

Sigh I could also say please pay attention to what I said also.

All that I meant by that is that there should be proper error handling. Not sure why you seem to think I meant getters and setters are bad (Even though I haven't once said that in this thread, and have been saying the complete opposite through this whole thread...).

You and me might not agree one that or this whole subject which is fine but there is no need to act condescending.

[BHX]

I would really appreciate if you read what I posted before, saying I advocated you use public members instead.

Might want to take your own advice on that. I have always used getters and setters and will continue to do so. Just curious why so many claim they are bad, but my last few posts have been asking how my code breaks encapsulation.

ne555 wrote:

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

ne555 said my code broke encapsulation, and I have been asking how it breaks it because my code keeps the members restricted to the class, which is the whole point of encapsulation. Yet when I ask how it breaks encapsulation, I get replies about getters/setters use, but at no point has anyone explained how my code breaks encapsulation.

Did the fixes for constants and removed the old comment just to save a few lines.

#include <iostream>
using namespace std;

class CRectangle {
    int x, y;
  public:
    void set_values (int,int);  // setter
    int getX() const { return x; } // getter
    int getY() const { return 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;
  // could have just done this and removed a lot of work
  cout << "area: " << area(rect.getX(), rect.getY());
  return 0;
}

[LB]

What if you change the type of x and y? If you can't without changing the accessors, you have broken encapsulation.

[TheIdeasMan]

@BHXSpecter

Because you have public get & set functions for all your class variables, it is the same as having all public variables.

Also, you have a generic area function which takes arguments. I thought part of the idea of encapsulation was that all the Methods & Properties for an object should go in the object's class, and other types of objects would have their own area function. So the area function for CRectangle would be this:

int CRectangle::Area() { return (x* y); }

With the set functions, wouldn't it be better to think about why the dimensions of your rectangle might change and provide functions that do that. For example, Scale or Stretch functions. These functions would alter the values of x & y and Area & Perimeter would still give the right answer. And if the rectangle had a coordinate in one corner, you could have Move & Rotate as well. These functions would have to behave appropriately.

This is a lot better than just having an arbitrary change in the dimensions.

Of course one should set the values initially with a constructor using an initialiser list.

@L B

What if you change the type of x and y?

I am interested to hear how you would do that? I am thinking with templates so that objects and their underlying data and return value objects can be built with any appropriate types. (Not that I know much about templates)

[S6k9GNh0]

Didn't read the entire things but...

Setters and getters can be useful if setting/getting an object does more than just set or get a variable. D has something they called user-defined properties you can set to remove the need to explicitly use getter/setter syntax and still get the same functionality. I actually like it:

http://dlang.org/property.html

[BHX]

LB wrote:

What if you change the type of x and y? If you can't without changing the accessors, you have broken encapsulation.

That again is a syntax error (clashing data types) and not breaking encapsulation. Encapsulation is the process of making the members (x, y) restricted to the class. Meaning no one outside the class methods can modify it. You keep telling me I'm breaking encapsulation and yet no one has shown it being broken. Are we talking about two different ways of interpreting encapsulation? Because I take it to mean keeping the data members private and only accessible to the class methods (ie int x, y; can only be directly accessed by set/get functions and other functions inside the class) while my code is getting a copy of the data in the class and then manipulating the copied data.

I think we need to come to an understanding of what encapsulation means as apparently the definition I'm going off of isn't what you are going off of.

[o1vk4iN6]

Who started using x and y to describe the width and height of a rectangle ? Why not just w, h and getWidth(), getHeight(). What if you want to add position to your rectangle, you would have to change everything lol. Great encapsulation there.

Also don't really like the set_values() function, if you modify the class and have to add more you have to change code, even if those values don't need to be modified. Also what if you have many variables:

// ...
    void set_values(int, int, int, int, int, int, int, int, int, int, int);
// ... 

Pretty easy to mistake setting the wrong value. Though having that many getter/setters for that many values would also be a pain.

[LB]

BHXSpecter wrote:

> What if you change the type of x and y? If you can't without changing the accessors, you have broken encapsulation.

That again is a syntax error (clashing data types) and not breaking encapsulation.

How is that a syntax error? If your class is design properly you can freely change the underlying types without ever having to change the public interface (which also allows for binary compatibility for those that absolutely need it).

[o1vk4iN6]

That doesn't really make sense for a rectangle, if you change the internal types to double why would you leave all the getters and setters as int ? That is assuming a project wide change, all your classes etc need to now be float. Unless you defined some sort of alias "scalar" to begin with and use that instead of int or w/e the interface will need changing.

[TheIdeasMan]

@L B

If your class is design properly you can freely change the underlying types without ever having to change the public interface (which also allows for binary compatibility for those that absolutely need it).

Can you show us how you would do that for the Rectangle class? That is change the type from int to any other integral type, or float, or double.

[cire]

If your class is design properly you can freely change the underlying types without ever having to change the public interface (which also allows for binary compatibility for those that absolutely need it).

Is it your assertion that a properly designed class is always properly designed in all contexts and never needs to be redesigned or refactored for reuse in another project (to include a future version of the same project) or to conform to a client's needs?

How is that a syntax error? If your class is design properly you can freely change the underlying types without ever having to change the public interface (which also allows for binary compatibility for those that absolutely need it).

If the client code hasn't changed to need a finer granularity represented by a double, for instance, there would be no reason to change the underlying type or interface. If the client code did change and we must change our design to cater to it. So what? The design specifies how the class interacts with the client. The interface had to change, regardless of the internals of the class, because the way the client code and our code interacts needed to change.

The point of encapsulation/information hiding is to keep internal changes to the class from affecting the interface of the class. In this case, the interface needed to be changed. Not because the internals needed to be changed (indeed we could have been using doubles internally all along without affecting the interface), but because the interaction between our code and the clients changed.

[LB]

@TheIdeasMan

Can you show us how you would do that for the Rectangle class? That is change the type from int to any other integral type, or float, or double.[/quote]You would literally just perform casts in the interface functions that deal with ints and not perform casts in the interface functions hat deal with floats.

[BHX]

How is that a syntax error?

Wrong term, but I was thinking of the error and warnings you get about type conversions.

[TheIdeasMan]

@L B

OK, as well as that, would you overload the interface functions so they take references to various types - that way avoiding being restricted to one return type?

void CRectangle::Area(std::size_t& TheArea){TheArea = static_cast<std::size_t>m_Area;}
void CRectangle::Area(float& TheArea){TheArea = static_cast<float>m_Area;}
void CRectangle::Area(double& TheArea){TheArea = m_Area;}

double MyArea = 0.0;
CRectangle MyRect;

MyRect.Area(MyArea);

Or alternatively, might it be better to use templates for everything? Which goes right back to the example Cubbi had shown us of the Boost Library Points, Lines etc, much earlier.

I keep think of a CAD (Computer Aided Drawing ) application, in which each type of Drawing Object needs access to all the other Drawing object types' data, because the user is frequently drawing from one object to another.

Having the templated get functions is great because one can choose exactly which data they want from the object. They may not want all of it for a complex object.

I am guessing that one might use a Mediator DP to restrict the coupling from each Drawing Object to the Mediator class, rather than to the entire application. But this probably means retrieving all the info for a particular object.

The other thing about CAD dwg objects is that IMO their underlying type will always be double, and I don't see a reason for that changing at any time. Sure there are selection boxes that have to have unsigned values to do with mouse co-ords, but that is known in advance & the GUI framework already has Rectangle classes for that type of purpose.

Hell, I think I am getting nearer to a complete understanding, after all this discussion 8<)

[LB]

Except you would never set the area.

[cire]

Never say never.

[TheIdeasMan]

@

L B

Hopefully we all agree that setting a Property with a direct assignment is really bad. The m_Area would always be a result of m_Width * m_Height, and MyArea is just a variable to put the answer in, rather than returning a value from a function.

[rapidcoder]

Can you show us how you would do that for the Rectangle class? That is change the type from int to any other integral type, or float, or double.

What's the problem? There might be a problem with changing to a float, but a double can hold any integer exactly. So you only change the field type and *leave* the getters and setters signatures as they were before, and you add casts from/to int where appropriate. Then you add two *new* better getters and setters for getting/setting doubles directly to be used by any new code. The reason for this is you are not breaking encapsulation and the outside world can still operate without any modifications nor recompilation using the old getters/setter.... This is called backward compatibility.

The other thing about CAD dwg objects is that IMO their underlying type will always be double, and I don't see a reason for that changing at any time.

Ehm, what? CAD using doubles as internal representation? Maybe toy drawing programs like Paint or 3D-Studio, but not CAD. CADs must use integer/decimal representation internally. Why? In architecture there is no reason for precision better than 1mm, but 0.1m must be 0.1m exactly and not some crappy 0.099999997m.

[TheIdeasMan]

@rapidcoder

Why? In architecture there is no reason for precision better than 1mm, but 0.1m must be 0.1m exactly and not some crappy 0.099999997m.

OK, Maybe I had the wrong impression. Perhaps I assumed: this sort of problem could be handled in the normal way with a scaled epsilon. AutoDesk quotes the precision of it's values as being 16sf which is about what one gets for a double.

I am guessing you have learnt this through your studies at University - which is good - so thanks for that.

However it is difficult for me to think of many situations where the precision of a double would be exceeded in most normal examples of using a CAD system. CAD systems tend to store nearly everything with coordinates, and things like dimension dwg objects & other things have an associated precision like 0.001m for example. Normal precision problems like bool PointInCircle(CPoint& ThePoint){} are handled in the normal way with the scaled epsilon. Btw, I am sure you realise the number you quoted is a float not a double.

Now that I think about it, the worst case scenario for Land Surveying is probably the use of Geodetic Distances & Line Scale Factors. LSF are near to 1.0 and are quoted to 8 dp and the usual maximum distance is about 60km with 1mm precision. So that is still only 8sf. The compiler does the calculation with 16 sf, but only the first 8 matter for the answer.

I have AutoDesk Civil3D software which a a Geodetic Calculator in it. It uses rigorous formulae that are within the precision of a double to calculate with.