Public setters have two huge problems: they expose the internal state of objects and therefore increase the coupling of the system over time. This might not be obvious to new developers or to experienced developers who haven’t worked in systems with strong encapsulation practices. Therefore it is a widely used practice that hurts software quality in the industry. Sometimes setters are useful, but there are major downsides to long term maintainability. Overall code becomes clearer and easier to work with over time if public setters are omitted.
The obvious problem
Say you have a very simple User class with a couple of properties, for the first name, last name and email:
@Setter // lombok annotation to automatically add setters to the object
class User {
String firstName;
String lastName;
String email;
public String getName() {
return firstName + " " + lastName;
}
}
You have an changeName action that changes the name. However, the name does not have the firstName/lastName split in the UI (but it is still needed in the domain):
class UserNameChanger {
Database db;
public void changeName(String id, String name) {
var user = db.getUserById(id);
var nameSplit = name.split(" ");
var firstName = nameSplit[0];
var lastName = String.join(" ", Arrays.copyOfRange(nameSplit, 1, nameSplit.length));;
user.setFirstName(firstName);
user.setLastName(lastName);
db.save(id, user);
}
}
Here it becomes obvious that the UserNameChanger contains domain logic that rightly belongs inside the User class, violating the principle of encapsulation. It is even more of a problem if this is not the only place a user can have their name changed. For example the same logic might be needed on creation and if the User is updated in a different context.
Instead the logic should be refactored to be on the User:
class User {
String firstName;
String lastName;
String email;
public String getName() {
return firstName + " " + lastName;
}
public void changeName(String name) {
var nameSplit = name.split(" ");
firstName = nameSplit[0];
lastName = String.join(" ", Arrays.copyOfRange(nameSplit, 1, nameSplit.length));;
}
}
Writing tests for the User is now very simple too, and it can be done without the database. The UserNameChanger can be simplified to:
class UserNameChanger {
Database db;
public void changeName(String id, String name) {
var user = db.getUserById(id);
user.changeName(name);
db.save(id, user);
}
}
This example shows logic being placed on an object that is not supposed to know about it. The setters are a problem, because without them the bad would be more obvious. By slapping a @Setter on the domain object it is possible to violate the state in a way that is not obvious to the developer. By discouraging public setters it is more obvious that the bounds are violated.
The less obvious problem
In some cases it might be perfectly valid to have a public setter. Take the following example with a User with only a single e-mail:
@Setter
class User {
String email;
}
The connected service to update the email is as follows:
class UserEmailChanger {
Database db;
public void changeEmail(String id, String email) {
var user = db.getUserById(id);
user.setEmail(email);
db.save(id, user);
}
}
This code is perfectly fine, but it problems appear when suddenly extra information has to be added. Say product management wants to add a new way of differentiating users and they want to be calculated based on the e-mail. Because the UserEmailChanger is the obvious place to put the code it might be done like this:
@Setter
class User {
String email;
UserType type;
}
class UserEmailChanger {
Database db;
public void changeEmail(String id, String email) {
var user = db.getUserById(id);
user.setEmail(email);
var userType = UserType.from(email);
user.setUserType(userType);
db.save(id, user);
}
}
Because the setter was already in place it began being used. If the User had instead had a changeEmail method, it would have been obvious that was the place the type should have been set:
class User {
String email;
UserType type;
public void changeEmail(String email) {
this.email = email;
userType = UserType.from(email);
}
}
In the example we started out with an initial use-case that was okay, but as soon as the requirements changed, code smells appeared.
DTOs and ViewModels
There is a case to be made that this is not a problem in DTOs and ViewModels, because they exist as pure data to be sent to an external system or UI. There is also a benefit to keeping them immutable, but that is another discussion that this post will not focus on.
Conclusion
By explicitly naming methods according to their purpose, object state is better encapsulated. This makes systems easier to test and more adaptable to changing requirements.