Don’t just create Setter methods, try creating operation methods

In Java, I come across a lot of unused “setter” and “getter” methods. Creating setters and getters often strikes roots with developers that work with Hibernate and Lombok. Hibernate needs the setters in order to set the values in the data model objects, and in Lombok it’s just adding an annotation, that maybe makes people lazy in deciding whether or not those setters or getters are needed.

This blog post is created after I saw a piece of code that someone wrote in my workplace — which made use of a setter, when it was unnecessary and caused code duplication and messy code.

I simplified things and changed the topic that the code relates to. Now the code is managing monitors (screens) in a company, telling if they are turned on (active = 1) or off (active = 0). The reason that active is integer and not boolean is that the developer wished to save the active state in the database in a compact way (e.g. instead of a string of “true” or “false” to use Boolean.valueOf).

This was the bad code that ended up in our system:

public class Monitor {
  private Integer active;
  public Integer getActive() {
    return active;
  }
  public void setActive(Integer active) {
    this.active = active;
  }
}
public class MonitorDto {
  private boolean active;
  //setter and getter here
}
public class MonitorService {
  public void saveMonitor(Monitor monitor) {
    //save the monitor to the database
  }
  public void activateMonitor(Long monitorId) {
    Monitor monitor = findMonitorById(monitorId); //say method exists
    monitor.setActive(1);
    saveMonitor(monitor);
  }
}
public class MonitorController {
  @Autowired
  private MonitorService monitorService;
  @PutMethod("/monitor/save")
  public String save(@RequestParam MonitorDto monitorDto) { 
    Monitor monitor = new Monitor();
    monitor.setActive(monitorDto.getActive() ? 1 : 0);
    monitorService.saveMonitor(monitor);
    return ... //something
    //skipping handling errors for code simplicity
  }
  @GetMapping("/monitor/activate/{id}")
  public String save(@PathVariable Long id) { 
    monitorService.activateMonitor(id);
    return ... //something
    //skipping handling errors for code simplicity
  }
}

If we refer to the active state (and its type) in a Monitor to be a “secret recipe” — then the main issue with this code is that 3 classes “know the secret recipe” for a monitor, which make it “not so secret”.
Those classes are: the data model itself (Monitor class), the service that manages it (MonitorService class) and the API for the world (MonitorController class).
The problem could have been worse, if we hadn’t had a DTO (MonitorDto) for our API (MonitorController). Without it, the logic of turning a boolean value into the relevant integer value could have been required from the client. Oh my…

There are better ways to approach this.

Solution 1: Keep it simple

A familiar solution would be to expose the logic of turning a boolean value for “active” Monitor, only in our data model (Monitor class). All other classes would not know of the “secret recipe” that the “active” state is saved as an integer — and would work with the tools the data model gave them — which is a setter of boolean type.

It would look like this:

public class Monitor {
 private Integer active;
 public boolean getIsActive() {
   return active ? 1 : 0;
 }
 public void setActive(boolean active) {
   this.active = active ? 1 : 0;
 }
}
public class MonitorDto {
 private boolean active;
 //setter and getter here
}
public class MonitorService {
 public void saveMonitor(Monitor monitor) {
   //save the monitor to the database
 }
 public void activateMonitor(Long monitorId) {
   Monitor monitor = findMonitorById(monitorId); //say method exists
   monitor.setIsActive(true);
   saveMonitor(monitor);
 }
}
public class MonitorController {
 @Autowired
 private MonitorService monitorService;
 @PutMethod("/monitor/save")
 public String save(@RequestParam MonitorDto monitorDto) { 
   Monitor monitor = new Monitor();
   monitor.setActive(monitorDto.getActive());
   monitorService.saveMonitor(monitor);
   return ... //something
   //skipping handling errors for code simplicity
 }
 @GetMapping("/monitor/activate/{id}")
 public String save(@PathVariable Long id) { 
   monitorService.activateMonitor(id);
   return ... //something
   //skipping handling errors for code simplicity
 }
}

As you can see, the “secret recipe” is “contained” only to the data model. That is relatively neat. But still the clients of our data model need to be aware that there are two states to “active”, and need to understand that TRUE means active, and FALSE means inactive. Pretty basic to understand in this simple example, but think of greater problems we want to expose methods (“internal” APIs) for — that can get hairy. Well, luckily, there’s a better way.

Solution 2: Borrow from DDD

We can borrow some concepts from Domain-Driven Design (DDD), to expose “operations” and disallow mutating existing models.
Basically this means that we don’t “blindly” expose a setter, but instead, we expose an easy to understand API.

It would look like this:

public class Monitor {
  private Integer active;
  public Monitor(active) {
    this.active = active ? 1 : 0;
  }
  
  public boolean getIsActive() {
    return active ? 1 : 0;
  }
  
  public void activate() {
    this.active = 1;
  }
  
  public void deactivate() {
    this.active = 0;
  }
}
public class MonitorDto {
 private boolean active;
 //setter and getter here
}
public class MonitorService {
 public void saveMonitor(Monitor monitor) {
   //save the monitor to the database
 }
public void activateMonitor(Long monitorId) {
   Monitor monitor = findMonitorById(monitorId); //say method exists
   monitor.activate();
   saveMonitor(monitor);
 }
}
public class MonitorController {
@Autowired
 private MonitorService monitorService;
@PutMethod("/monitor/save")
 public String save(@RequestParam MonitorDto monitorDto) { 
   Monitor monitor = new Monitor(monitorDto.getActive());
   monitorService.saveMonitor(monitor);
   return ... //something
   //skipping handling errors for code simplicity
 }
@GetMapping("/monitor/activate/{id}")
 public String save(@PathVariable Long id) { 
   monitorService.activateMonitor(id);
   return ... //something
   //skipping handling errors for code simplicity
 }
}

The cool thing here is we have the simplest API possible: If the data model’s client wants to active, he simply uses Monitor.activate() and to deactivate — he simply uses Monitor.deactivate(). This way no logic exits Monitor class. It is the only one knowing how the “secret recipe” of “active” state data-type is stored.

Oh… BTW… Not “auto creating” setters and thinking before creating them or even the “operations” I introduced in Solution 2, will make it easier for you to create immutable classes or immutable fields. If Monitor class would have a field named “model” that doesn’t ever change — then it could’ve been an immutable field (with the “final” keyword).