Yes, getters/setters do break encapsulation - they basically are just an extra layer between directly accessing the underlying field. You might as well just access it directly.
Now, if you want more complex methods to access the field, that's valid, but instead of exposing the field, you need to think what methods the class should be offering instead. ie. instead of a Bank class with a Money value that is exposed using a property, you need to think what types of access a Bank object should offer (add money, withdraw, get balance) and implement those instead. A property on the Money variable is only syntactically different from exposing the Money variable directly.
DrDobbs has an article that says more.
For your problem, I'd have 2 methods setStyle and clearStyle (or whatever) that take an enum of the possible styles. A switch statement inside these methods would then apply the relevant values to the appropriate class variables. This way, you can change the internal representation of the styles to something else if you later decide to store them as a string (for use in HTML for example) - something that would require all users of your class to be changed too if you used get/set properties.
You can still switch on strings if you want to take arbitrary values, either have a big if-then statement (if there's a few of them), or a map of string values to method pointers using std::mem_fun (or std::function) so "bold" would be stored in a map key with its value being a sts::mem_fun to a method that sets the variable bold to true (if the strings are the same as the member variable names, then you can also use the stringifying macro to reduce the amount of code you need to write)
What you describe is known as an anemic domain model. As with many OOP design principles (like Law of Demeter etc.), it's not worth bending over backwards just to satisfy a rule.
Nothing wrong about having bags of values, as long as they don't clutter the entire landscape and don't rely on other objects to do the housekeeping they could be doing for themselves.
It would certainly be a code smell if you had a separate class just for modifying properties of Card
- if it could be reasonably expected to take care of them on its own.
But is it really a job of a Card
to know which Player
it is visible to?
And why implement Card.isVisibleTo(Player p)
, but not Player.isVisibleTo(Card c)
? Or vice versa?
Yes, you can try to come up with some sort of a rule for that as you did - like Player
being more high level than a Card
(?) - but it's not that straightforward to guess and I'll have to look in more than one place to find the method.
Over time it can lead to a rotten design compromise of implementing isVisibleTo
on both Card
and Player
class, which I believe is a no-no. Why so? Because I already imagine the shameful day when player1.isVisibleTo(card1)
will return a different value than card1.isVisibleTo(player1).
I think - it's subjective - this should be made impossible by design.
Mutual visibility of cards and players should better be governed by some sort of a context object - be it Viewport
, Deal
or Game
.
It's not equal to having global functions. After all, there may be many concurrent games. Note that the same card can be used simultaneously on many tables. Shall we create many Card
instances for each ace of spade?
I might still implement isVisibleTo
on Card
, but pass a context object to it and make Card
delegate the query. Program to interface to avoid high coupling.
As for your second example - if the document ID consists only of a BigDecimal
, why create a wrapper class for it at all?
I'd say all you need is a DocumentRepository.getDocument(BigDecimal documentID);
By the way, while absent from Java, there are struct
s in C#.
See
for reference. It's a highly object-oriented language, but noone makes a big deal out of it.
Best Answer
As stated in quite a few answers and comments, DTOs are appropriate and useful in some situations, especially in transferring data across boundaries (e.g. serializing to JSON to send through a web service). For the rest of this answer, I'll more or less ignore that and talk about domain classes, and how they can be designed to minimize (if not eliminate) getters and setters, and still be useful in a large project. I also won't talk about why remove getters or setters, or when to do so, because those are questions of their own.
As an example, imagine that your project is a board game like Chess or Battleship. You might have various ways of representing this in a presentation layer (console app, web service, GUI,etc.), but you also have a core domain. One class you might have is
Coordinate
, representing a position on the board. The "evil" way to write it would be:(I'm going to be writing code examples in C# rather than Java, for brevity and because I'm more familiar with it. Hopefully that's not a problem. The concepts are the same and the translation should be simple.)
Removing Setters: Immutability
While public getters and setters are both potentially problematic, setters are the far more "evil" of the two. They're also usually the easier to eliminate. The process is a simple one- set the value from within the constructor. Any methods which previously mutated the object should instead return a new result. So:
Note that this doesn't protect against other methods in the class mutating X and Y. To be more strictly immutable, you could use
readonly
(final
in Java). But either way- whether you make your properties truly immutable or just prevent direct public mutation through setters- it does the trick of removing your public setters. In the vast majority of situations, this works just fine.Removing Getters, Part 1: Designing for Behavior
The above is all well and good for setters, but in terms of getters, we actually shot ourselves in the foot before even starting. Our process was to think of what a coordinate is- the data it represents- and create a class around that. Instead, we should have started with what behavior we need from a coordinate. This process, by the way, is aided by TDD, where we only extract classes like this once we have a need for them, so we start with the desired behavior and work from there.
So let's say that the first place you found yourself needing a
Coordinate
was for collision detection: you wanted to check if two pieces occupy the same space on the board. Here's the "evil" way (constructors omitted for brevity):And here's the good way:
(
IEquatable
implementation abbreviated for simplicity). By designing for behavior rather than modelling data, we've managed to remove our getters.Note this is also relevant to your example. You may be using an ORM, or display customer information on a website or something, in which case some kind of
Customer
DTO would probably make sense. But just because your system includes customers and they are represented in the data model does not automatically mean you should have aCustomer
class in your domain. Maybe as you design for behavior, one will emerge, but if you want to avoid getters, don't create one pre-emptively.Removing Getters, Part 2: External Behaviour
So the above is a good start, but sooner or later you will probably run into a situation where you have behavior which is associated with a class, which in some way depends on the class's state, but which doesn't belong on the class. This sort of behavior is what typically lives in the service layer of your application.
Taking our
Coordinate
example, eventually you'll want to represent your game to the user, and that might mean drawing to the screen. You might, for example, have a UI project which usesVector2
to represent a point on the screen. But it would be inappropriate for theCoordinate
class to take charge of converting from a coordinate to a point on the screen- that would be bringing all sorts of presentation concerns into your core domain. Unfortunately this type of situation is inherent in OO design.The first option, which is very commonly chosen, is just expose the damn getters and say to hell with it. This has the advantage of simplicity. But since we're talking about avoiding getters, let's say for argument's sake we reject this one and see what other options there are.
A second option is to add some kind of
.ToDTO()
method on your class. This- or similar- may well be needed anyway, for example when you want to save the game you need to capture pretty much all of your state. But the difference between doing this for your services and just accessing the getter directly is more or less aesthetic. It still has just as much "evil" to it.A third option- which I've seen advocated by Zoran Horvat in a couple of Pluralsight videos- is to use a modified version of the visitor pattern. This is a pretty unusual use and variation of the pattern and I think people's mileage will vary massively on whether it's adding complexity for no real gain or whether it's a nice compromise for the situation. The idea is essentially to use the standard visitor pattern, but have the
Visit
methods take the state they need as parameters, instead of the class they're visiting. Examples can be found here.For our problem, a solution using this pattern would be:
As you can probably tell,
_x
and_y
aren't really encapsulated any more. We could extract them by creating anIPositionTransformer<Tuple<int,int>>
which just returns them directly. Depending on taste, you may feel this makes the entire exercise pointless.However, with public getters, it's very easy to do things the wrong way, just pulling data out directly and using it in violation of Tell, Don't Ask. Whereas using this pattern it's actually simpler to do it the right way: when you want to create behaviour, you'll automatically start by creating a type associated with it. Violations of TDA will be very obviously smelly and probably require working around a simpler, better solution. In practice, these points make it much easier to do it the right, OO, way than the "evil" way that getters encourage.
Finally, even if it isn't initially obvious, there may in fact be ways to expose enough of what you need as behavior to avoid needing to expose state. For example, using our previous version of
Coordinate
whose only public member isEquals()
(in practice it would need a fullIEquatable
implementation), you could write the following class in your presentation layer:It turns out, perhaps surprisingly, that all the behavior we really needed from a coordinate to achieve our goal was equality checking! Of course, this solution is tailored to this problem, and makes assumptions about acceptable memory usage/performance. It's just an example that fits this particular problem domain, rather than a blueprint for a general solution.
And again, opinions will vary on whether in practice this is needless complexity. In some cases, no such solution like this might exist, or it might be prohibitively weird or complex, in which case you can revert to the above three.