6

In Clean Code:

public class Point {
   public double x;
   public double y;
}

The author wrote about the above Point() class:

This exposes implementation. Indeed, it would expose implementation even if the variables were private and we were using single variable getters and setters.

This means I must burn my school copybooks.

Why is combining getters, setters and private access modifier not enough?

  • 12
    "This means I must burn my school copybooks." – Probably. OOP is usually taught very badly. – Jörg W Mittag Apr 24 '17 at 9:14
  • 4
    I'm not sure what the author is getting at. If the purpose of a Point object is to represent a pair of cartesian coordinates, then exposing x and y properties is exposing functionality, not implementation details. Functionality is exactly what we want to expose. – Tanner Swett Apr 24 '17 at 11:37
  • 3
    The purpose of a Point or 2d vector abstraction is to make it easy to translate mathematical formulas into code, not to hide the results of those formulas. Encapsulation only applies to those outputs that should not be public. – Frank Hileman Apr 24 '17 at 21:12
  • 2
    @BillalBEGUERADJ: "Common" is a poor way to identify useful beliefs. – Robert Harvey May 5 '17 at 16:54
  • 6
    When I look at the example the OP is trying to understand, warning bells go off in my head. I think some of the real issue is that the author tried to boil down a complicated concept too far, and got left with nothing. The concept of a Point is so simple that it actually doesn't demonstrate the ideal at all. It's like what happens when you try to boil down how to live a moral life by discussing the moral obligations of a rock. – Cort Ammon May 5 '17 at 22:55

10 Answers 10

6
+100

The example given, of a Point class with two public members of type double named x and y, exposes its implementation by allowing anyone who has an object of type Point to know about its implementation. In this case, the entire implementation is exposed.

If you changed the public members to private and created a pair of getters and setters, you still expose its implementation. If you look at the interface of an object and see that it has methods like double getX(), double getY(), setX(double x), and setY(double y), then you aren't going to be sitting there thinking, "Gee, I wonder how they implemented that. I can't tell."

The second version of Point isn't really different from the first one, except that in most languages it will have a lot of unnecessary boilerplate. In both cases, looking at the object's interface tells you about how it is implemented.

If we wanted to change the internal representation of a Point to polar coordinates, we run into problems. We can still use the old interface, but it's clunky. A call to setX() is going to have to do an internal conversion to rectangular coordinates, change x, and then convert back to polar, introducing unnecessary rounding errors and extra computations.

If we had started out by designing an interface for a Point without reference to an implementation, we wouldn't have that sort of problem. Say we decide that a point has an x and y coordinate, and also an angle and magnitude. We could have double x(), double y(), double angle(), and double magnitude() methods. We could make them immutable, but let's say we didn't. So we need rectangular(double x, double y) and polar(double angle, double magnitude) to move the point.

What do we know about the implementation? Well, we know something, since a two-dimensional point is a very simple thing, but we don't know whether the object's internal representation is in polar or rectangular coordinates. The internal representation might even include both polar and rectangular coordinates, either with booleans to keep track of whether a particular set of coordinates is up to date, or else forcing everything to always be up to date automatically.

So in this case the implementation isn't exposed, since we can only guess how it was implemented.

  • I do not understand why your answer is not upvoted. Only your answer and that of Erik addressed my question. Other answers discussed things I did not ask for. I read the whole book I mentioned and I understood the statement I quoted almost exactly as you described it here. I wish I could divide the bounty between you and Erik but since I can not and your answer seems to clearly read in the author's mind, I opt to reward it for you. Thank you very much for the effort. I also thank all other members for their feedback. – Billal Begueradj May 8 '17 at 6:55
  • @BillalBEGUERADJ: well, I think I understand perfectly what happened here: your question is IMHO very unclear, and it provokes guessing around about the issues you have with the cite from "Clean Code". This answer just guessed correctly what you meant, JörgWMittag guessed probably wrong and tried to give you a more broader, theoretical answer. – Doc Brown May 8 '17 at 9:58
  • When I started the bounty, I clearly stated through its description I read the existing answers including the full 2 PDF files provided by the user you mentioned. I carefully read all the answers and upvoted all of them (by principle, I upvote any person who answers to me even when I do not agree with him/her because that is a way for me to show that I respect them for their effort). But really I find this answer helps me to see the things better. I apologize from other users who answered me for not being enough clear through my question. @DocBrown – Billal Begueradj May 8 '17 at 10:40
  • @BillalBEGUERADJ: don't mind, IMHO your question, though probably not beeing crystal-clear, motivated some nice answers, and it is perfectly ok not to accept the one with the most upvotes. But you asked why this answer did not get more upvotes, so I tried to give you an explanation. – Doc Brown May 8 '17 at 11:14
  • @Billal BEGUERADJ: I think the reason it wasn't upvoted much has to do mostly with visibility. The question was originally asked 14 days ago, and I didn't see it until 2 days ago, so for 12 days, anyone reading the question and its answers couldn't have upvoted my answer because it didn't exist yet. After that, it was one of many answers, some long, and it was sorted towards the bottom by default. – Michael Shaw May 8 '17 at 19:53
27

Object-Oriented Data Abstraction is all about Behavioral Abstraction. The implementation details are hidden behind a behavioral interface, i.e. an interface that does something.

A variable is not behavior, it is state. A getter or setter is just a variable in disguise.

The question is: "what does a Point do", not "what state does it have".

For example, how would you compute a new point from an existing point? You have no choice but to "take apart" the point, do all the necessary calculations yourself, then put the point back together again. And you have to do this in every single piece of code that wants to somehow create new points from old ones. And what happens if you decide that representing points as cartesian coordinates is inefficient and you would rather represent it using polar coordinates?

But, what if the point wasn't just a "dead" bag of data? What if the point had behavior? What if it knew itself how to construct a new point from itself? For example, we can get a new Point by adding a displacement vector. So, we add an add method to the point which takes a displacement vector as a parameter (example in Scala, but it doesn't matter):

class Point(val x: Double, val y: Double) {
  def +(v: Vector) = ???
}

Now, any piece of code can create a new point by taking an existing point and adding a displacement vector to it, without having to know anything about how Points or Vectors work. You can freely change the implementation and representation of either or both, without affecting any other code.

If you know a bit of Scala, you may have noticed that I had Scala generate automatic getters for x and y. I feel that's okay, because you sometimes do want to work with them separately. I did, however, not generate setters!

If we take something more complex, like a collection interface, it becomes even more apparent. How would you rather traverse a linked list:

// without abstraction:
var node = list.head
while (node != null) {
  println(node)
  node = node.next
}

// with abstraction:
list.foreach(println)

Not only is the code using the abstracted behavioral interface much simpler, it also works for any collection, not just for lists. The collection data structure itself can decide how to execute the traversal most efficiently, for example, it could decide to execute it in parallel on multiple CPU cores.

There is a better example of this principle in action in the seminal paper On Understanding Data Abstraction, Revisited by William R. Cook, in which Cook explains the fundamental difference between Object-Oriented Data Abstraction and Data Abstraction based on Abstract Data Types. Unfortunately, many textbooks treat Objects as only a slight variation of ADTs (e.g. "Objects = ADTs + Inheritance"), or even one and the same, but they are in fact fundamentally different. On the Criteria To Be Used in Decomposing Systems into Modules by David Parnas is also still a good read, even 45 years later.

8

Like everything else in our highly abstract discipline, this maxim (hide implementations) is a matter of degree.

In this case, you can only follow the maxim as rigidly as you imagine if your program actually doesn't need to manipulate X and Y coordinates. Perhaps it's enough to receive location data from a GPS module and distribute them to some update service or other without ever reasoning about the actual latitude or longitude of anything. In that case it's reasonable to create a class (or to handle an existing class under an interface) that does, in fact, not expose the fact that these cordinates exist.

But if the task of your program or module is to reason about latitude and longitute, then there probably is no way of not accessing these fine-grained data. In that case, trying to hide them is just counter-productive.

  • 3
    I think this is the right answer.. We're in danger in this thread of heading off into some sort of bizarre software solipsism. Encapsulation is the aim, but at some point all classes must expose something of themselves, otherwise they are useless. And in Java, in fact the beauty of java is that nothing is ever really truly hidden, because of reflection or byte code manipulation etc. A marker interface is the only real way of hiding implementation. Point could in theory implement an interface with no methods. Called 'Item' etc. – Richard May 1 '17 at 23:56
  • @Richard, This answer is not complete. It only points out that "Point" is an ambiguous name that can mean different things in different applications. Sometimes, we say "Point" when creating a low-level abstraction where we really need Cartesian coordinates (e.g., to address pixels on a screen), and other times it is valuable to think of "Point" at a higher level. Jörg Mittag's answer explains how it limits the utility of your code if you mix high-level and low-level abstractions together. – Solomon Slow May 7 '17 at 18:59
  • Perhaps point needs an enum constructor indicating cartesian..polar.. etc. But in 2d, your always going to need a 'point' object that manages 2 member variables – Richard May 7 '17 at 22:27
8

Well I'm coming late to this party, but I can't resist putting my two cents in.

Why is combining getters, setters and private access modifier not enough?

Not enough for what? If combining getters, setters and private access modifiers was a completely bad idea it would have died already. It's still around. It just doesn't do what people were told it does. It does something else.

This exposes implementation. Indeed, it would expose implementation even if the variables were private and we were using single variable getters and setters.

Uncle Bob is taking a shot at the myth that getters and setters provide encapsulation. They don't. They never did. We were told they did. We were lied to. We were fooled because we actually got something good outta the deal. It just wasn't encapsulation.

So what did we get?

We got Aspect Oriented Programming.

Before the languages, plugins, and frameworks provided for Aspect Oriented Programming, before that paradigm existed, getters and setters provided a very important thing to the world of programming. They provided a place to put a break point.

Yes, getters and setters are really debugging code. Sure you can tack on some validation magic but their biggest impact was to help with debugging. Leaving debugging code lying around is a sin though, so we kept this a dark secret.

Unfortunately, the secret corrupted our understanding of a very important concept. Encapsulation isn't simply preventing direct access. It's preserving the right to not know.

The moment you name a function getX() you've exposed the idea of X. You promised to go get an X. How you implement getX() doesn't really matter. That isn't the problem. The problem is right there in the name.

What business does any other object have even knowing about the idea of an X? A pure Behavior Object takes an idea and hides it so nothing else has to think about it. If you have an issue with X this Behavior Object should be the only place you have to look to fix that issue. If you let any ol` thing ask about X you're going to have a lot of places to look to fix that X issue.

Does this mean getters and setters are evil and wrong and should never be used again?

No. It means you should learn there is more in this world of programming than pure Behavior Objects. There are also Value Objects, Data Transfer Objects, Abstract Data Types, and Collections. Not to mention strings and int's. Call them what you like but don't get these mixed up with Behavior Objects. They follow different rules.

Behavior Objects do something none of these others do. They move you from one level of abstraction to another. If I can set X and then get X I haven't changed the level of abstraction at all.

If I can say Point(x, y).draw() I've moved to another level of abstraction.

If I have to say draw(p.getX(), p.getY()) then the point didn't change the level of abstraction at all from when it was given x and y. It's just a data dumping ground and it's making drawing something else's problem.

Behavior objects shouldn't be asked questions about their state. They should be told what do to. This principle is called Tell, don't ask.

So why shouldn't everything be a Behavior Object? Well believe me people have tried. But collections are just so darn useful. Sometimes you have work to do that DOESN'T move from one level of abstraction to another. Sometimes you just need to sort something.

I'll leave you with this:

Setters make you mutable. They also let old frameworks initialize objects in stages. Which means you can exist when you're not done being initialized.

Getters turn a behavior object inside out. If idea X has a getter than it's anyone's guess where your X related bug is hiding.

What they are good for is improving dumb data objects that otherwise would just have public state. If you can get away from using those, more power to you. But if you're going to use them then getters and setters will at least let me set breakpoints.

The very idea of getters and setters them came from the Java Bean. It was a specific idea that solved a specific problem that led many designers to use it in ways that remind me of a monkey driving a nail with a lug wrench. It can work but it's painful to watch.

This means I must burn my school copybooks.

If you're studying a subject where the ideas in the books will always be as true today as the day it was written you aren't studying science. You're studying religion.

Don't burn your old books. Keep them around for their historical perspective. They all tell you the date they were published. They'll come in handy when you realize the code you're reading was written before you were born.

7

One simple answer to the question "why does this expose implementation?" is that if you have two getters/setters that each return a double and are called X and Y, you are exposing that you've implemented the Point class using Carthesian Coordinates rather than, say, Polar Coordinates.

Also, giving people access to these X and Y values might lead them to manipulate them in calculations. For example; rather than using

Point->distanceTo(otherPoint);

They might do something like:

distance = sqrt(
    pow( PointA->getX() - PointB->getX(), 2 )
  + pow( PointA->getY() - PointB->getY(), 2 )  );

In addition to the code duplication that will inevitably happen, this makes a second assumption. This formula only works on a flat, rectangular plane. If you're actually representing a hexagon grid for example, then this function is simply wrong.

By not exposing how you're implementing a point on a field, you can later change between ways of storing these coordinates and you prevent people from trying to be clever with details they shouldn't concern themselves with in the first place.

  • Good answer but I think what's missing is an example of how to do it differently. – JimmyJames May 1 '17 at 16:41
  • 2
    Also, a good example of the distance issue is using the Pythagorean approach to find distances on a sphere or spheroid. You will get the wrong answer if you used this for large distances on the surface of the earth. – JimmyJames May 1 '17 at 16:43
2

I simply don't agree with the proposition. It depends on the semantic of the object you represent.

Your Point should be a value object (like an integer, ...), something entirely defined by it's state. You can have two different variable containing the number 1, but the number 1 is intrinsically unique, as is the Point (0,1). That class should be immutable, so having getters for it's X And Y coordinates is fine, as well as getters for it's polar coordinates R and Thêta. The internal representation could be either.

A class representing a Cup could have a property representing it's position. As I'm perfectly able to move a Cup arbitrarily, having a getter and a setter is fine. There could be some behavior, like automatically adjusting it's Height when it is dropped (when IsInMyHand becomes true).

On the other hand, a class representing a Car shouldn't have a setter for it's position (unless you're modeling a big crane, for example). It's position should depend on the history of it's GasPedal and SteeringWheel.

My answer to your question

Over simplification ?

1

There is an idiom to hide the private data. There are various names for it: Opaque Pointer, Pimpl Idiom, Cheshire Cat, etc. See https://en.wikipedia.org/wiki/Opaque_pointer for more details.

I read about it the first time in Large-Scale C++ Software Design by John Lakos. He used the term insulation to describe the practice. Insulation is stronger than encapsulation as far as how much control the developer has to modify the details of the private data.

Re:

Why the author says that combining getters, setters and private access modifier is not enough?

It is not enough to hide the implementation details of the public getter and setter functions. It is not enough if you, the developer of Point, want more freedom in how the private data is represented and would like the flexibility of changing the representation without impacting the users of Point.

Re:

then how to do it (hide an object's implementation details)?

I don't know the language of your posted snippet of code but in C++, it could be something like:

The header file, call it Point.h:

class Point {
   public:

      // Constuctor
      Point();

      // Destuctor
      ~Point();

      // Getter functions.
      double get_x() const;
      double get_y() const;

      // Setter functions.
      void set_x(double x);
      void set_y(double y);

      // Need to add copy constructor and copy assignment operators too.
      // Need to take care of the Rule of Three.

      // Class to hold the data
      class Data;

  private:

      // Pointer to the object holding private data
      Data* dataPtr;
};

The implementation file, call it Point.cpp:

#include "Point.h"

class Point::Data
{
   public:

      Data() : x(0), y(0) {}

      double x;
      double y;
};

Point::Point() : dataPtr(new Data()) {}

Point::~Point() { delete dataPtr; }

double Point::get_x() const { return dataPtr->x; }
double Point::get_y() const { return dataPtr->y; }

void Point::set_x(double x) { dataPtr->x = x; }
void Point::set_y(double y) { dataPtr->y = y; }

This completely hides how the private data of Point are stored. One of the interesting things that results from using this idiom in C++ is that the details of Data can be modified at will without requiring any of the files that depend on Point.h to be recompiled. This has real benefits in large applications with hundreds of .cpp files.

1

As mentioned by @Erik, the interface exposes the fact that it's implemented using the Cartesian coordinate system.

This is only a theoretical argument, as sometimes it's inappropriate not to design points as simple (x,y), for such reasons as instant familiarity and performance. But extending the theoretical argument, you could say that it's a CoordinateSystem's function to give you a numeric description of the location of a particular point. Just an an example, you could use double dispatch to convert a point from one coordinate system to another. E.g. in here.

interface Point {
    Point convertTo(CoordinateSystem cs);
}

class CartesianPoint implements Point {
    CartesianPoint(double x, double y) {
        this.x = x;
        this.y = y;
    }

    public Point convertTo(CoordinateSystem cs) {
        return cs.convert(this);
    }

    double x;
    double y;
}

class PolarPoint implements Point {
    public PolarPoint(double angle, double radius) {
        this.angle = angle;
        this.radius = radius;
    }

    public Point convertTo(CoordinateSystem cs) {
        return cs.convert(this);
    }

    double angle;
    double radius;
}

interface CoordinateSystem {
    Point convert(CartesianPoint p);
    Point convert(PolarPoint p);

    double distance(Point p1, Point p2);
}

class CartesianCoordinateSystem implements CoordinateSystem {
    public Point convert(CartesianPoint p) {
        return p;
    }

    public Point convert(PolarPoint p) {
        double x = p.radius * Math.cos(p.angle);
        double y = p.radius * Math.sin(p.angle);
        return new CartesianPoint(x, y);
    }

    public double distance(Point p1, Point p2) {
        CartesianPoint cp1 = (CartesianPoint)p1.convertTo(this);
        CartesianPoint cp2 = (CartesianPoint)p2.convertTo(this);

        double a = cp1.x - cp2.x;
        double b = cp1.y - cp2.y;
        return Math.sqrt((a * a) + (b * b));
    }
}

class Program {
    public static void main(String[] args) {
        PolarPoint p1 = new PolarPoint(1.23, 5);
        CartesianPoint p2 = new CartesianPoint(4, 6);
        CartesianCoordinateSystem cs = new CartesianCoordinateSystem();

        CartesianPoint cp1 = (CartesianPoint)cs.convert(p1);
        System.out.println("P1=(" + cp1.x + "," + cp1.y + ")");
        System.out.println("P2=(" + p2.x + "," + p2.y + ")");
        System.out.println("DISTANCE=" + cs.distance(p1, p2));
    }
}

That's the basic point (pun intended).

1

Making the class attributes private does hide implementation and providing public functions like double getX() does not expose implementation details per se.

The problem is that everyone will expect this function to be implemented like this:

double getX() { return this.x; }

and not, for example, like this:

double getX() { return this.radius * Math.cos(this.angle); }

The function name favours a specific implementation.

For that reason I try to avoid function names starting with get or set as much as possible.

If you need to extract the cartesian X coordinate of a point, you could name the function like this:

double extractCartesianX() { ... }

This name does not imply that X is actually a class attribute. It could be, but it could also be a calculated value.

Note: I tried to provide a brief answer that directly addresses your question. For more details and background information, you should check the great answer by @CandiedOrange.

1

The author of some book said: "This exposes implementation. Indeed, it would expose implementation even if the variables were private and we were using single variable getters and setters." The author is absolutely right. But in this case, the reply is: "So what?"

The Point class represents a point made up of two double precision coordinates. The user of the Point class can both read and assign each coordinate individually. That is an entirely approprate interface a class representing a mathematical point. Sure, the implementation is exposed, but in this case that is absolutely fine.

Exposing the implementation is bad if changing the implementation, while keeping the interface intact, would cause problems because the implementation was exposed. In this case, the interface, not the implementation, is two double precision instance variables directly accessible. So I can't quite see how anyone could change the implementation without changing the interface.

I would say that the book author took an awful example (not that the Point class is awful, it's absolutely fine, it's using it as an example for the evils of exposing an implementation that's awful). He or she should have used an example where exposing the implementation actually causes problems.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.