Lombok - the wrong way

I’ve been using the Scala language for a couple of years, yet I decided to join a new project written purely in Java as I was curious what new and interesting had happened in the Java ecosystem during these years. It’s also a totally new kind of problems to solve as the main purpose of the project is to help companies resolve the Vehicle Routing Problem — VRP in short.

Also, the project is built on top of Spring Boot with Lombok library which I haven’t been using for a very long time. And this story is about Lombok, at least part of it :)

Background

As you might know, Lombok is used to simplify Java development, to avoid playing with most of the boilepart in Java like getters/setters, constructors, builders, and so on.

On the first glance, it looks like a silver bullet to all the problems that have been reported by the Java community for years. With a single annotation, you can save a lot of time instead of manually writing all these setters/getters, implementing proper constructors or digging StackOverflow how to implement hashCode and equals in the right way.

Here is a simple example how Lombok saves your time (this is a real example from the project I’m working right now):

@Data
@Builder
@Jacksonized
@JsonNaming(PropertyNamingStrategies.SnakeCaseStrategy.class)
public class Job {
  public enum Type {
    DELIVERY,
    PICKUP;

    @JsonValue
    @Override
    public String toString() {
      return super.toString().toLowerCase();
    }
  }

  private long id;
  private Type type;
}

With just a few annotations on top of the class, you get a full-blown class with JSON support. You can also use a builder to construct instances of the class in a clean and efficient way. Let’s see how this class is used:

return stop.getOrders().stream()
  .map(order ->
    Job.builder()
      .id(order.getId())
      .type(                                       
        order.getDeliveryLocation().equals(stop.getLocation())
          ? Job.Type.DELIVERY
          : Job.Type.PICKUP)
      .build())
  .collect(Collectors.toList());

So you can use the builder to simply create a new instance, you can use fluent API to have easy to read code.

At first glance, everything seems right, doesn’t it?

Wrong!

I’ll just focus on the one aspect here — the Builder annotation. It gives me the .builder() method that returns an instance of the Builder class with dedicated methods to set proper values for each field defined in the Job class.

Yet, if you take a closer look, you will notice a line with enum Type definition. This enum tells me what kind of Job objects I can create — either PICKUP or DELIVERY. Yet the Builder doesn’t pursuit so, check the code below:

Job.builder().id(1L).build()

Does this create a proper instance? What type of job did I create? Is the Job instance valid?

As you can see, all the questions are valid and you cannot acknowledge them. What the above code built is totally invalid object — from the business point of view to be clear, but we do implement a business logic and not just code that should look nice.

Sure, you can complain that this is a developer’s fault, they should know what they are doing, they should know all those inner dependencies between id and type — this isn’t API responsibility to be clear and helpful … right ;-)

Also, using the Builder annotation this way, you do nothing than this:

var job = new Job();
job.setId(1L);
job.setType(PICKUP);
return job;

Just because you hide this, it doesn’t mean you create a good quality code.

Thinking

I think (therefore I am ;-) ) most of developers using Lombok these days stopped thinking about the domain they are modeling. They just throw a set of annotations into a class and go further, it sounds like they are paid by the line of code they are producing.

Lombok isn’t a hammer to nail all the problems, it’s a tool as any other and it’s our (developers') responsibility to use it properly.

How can I improve this code? The answer is easy: take a look on it from business perspective, what does the domain expect to look like?

If you take look from such an angle, you’ll notice that you can create two kind of jobs: either a pickup job or a delivery job. There is no option to create any other kind of job right now (this can change in the future, but that’s another story to tell).

Having this in mind the only changeable thing is the id which uniquely identifies a job object. So we can model the class with two static constructors to only allow create two kind of jobs:

@Data
@AllArgsConstructor(access = AccessLevel.PRIVATE)
@Jacksonized
@JsonNaming(PropertyNamingStrategies.SnakeCaseStrategy.class)
public class Job {
  public enum Type {
    DELIVERY,
    PICKUP;

    @JsonValue
    @Override
    public String toString() {
      return super.toString().toLowerCase();
    }
  }

  public static Job pickup(long id) {
    return new Job(id, Type.PICKUP);
  }

  public static Job delivery(long id) {
    return new Job(id, Type.DELIVERY);
  }

  private long id;
  private Type type;
}

Just by adding two factory methods (yep, there is such a design pattern) — pickup() and delivery() — and by making constructor private I limited how this class can be constructed. This also simplifies the usage of the class:

return stop.getOrders().stream()
  .map(order -> 
    order.getDeliveryLocation().equals(stop.getLocation())
      ? Job.delivery(order.getId())
      : Job.pickup(order.getId())
    )
  .collect(Collectors.toList());

With the new approach, I was able clearly expose what kind of objects a user of my API can create, what is needed to create those objects and just left two entry points into the API. User of the API doesn’t need to know if there is any dependency between id and type.

Another thing that shows up is the enum — it doesn’t have to to be exposed at all, it’s an internal Job‘s thing that should be hidden. How to do it?

@Data
@AllArgsConstructor(access = AccessLevel.PRIVATE)
@Jacksonized
@JsonNaming(PropertyNamingStrategies.SnakeCaseStrategy.class)
public class Job {
  private enum Type {
    DELIVERY,
    PICKUP;

    @JsonValue
    @Override
    public String toString() {
      return super.toString().toLowerCase();
    }
  }

  public static Job pickup(long id) {
    return new Job(id, Type.PICKUP);
  }

  public static Job delivery(long id) {
    return new Job(id, Type.DELIVERY);
  }

  private long id;
  private Type type;

  public boolean isPickup() {
    return Type.PICKUP.equals(this.type);
  }

  public boolean isDelivery() {
    return Type.DELIVERY.equals(this.type);
  }
}

I just made the enum Type private and exposed the internal state by two helper methods — isPickup() & isDelivery(). Now if someone would like to check what kind of job is an instance, they can use the helper methods:

var job = ...
if (job.isPickup()) {
   // do the pickup job
} else if (job.isDelivery()) {
  // do the delivery job
} else throw new IllegalStateException();

It wouldn’t be possible to discover this without first stop using Lombok to model my domain.

There is even more, the above approach exposes another case: Job is an interface and you can have two implementations: PickupJob and DeliveryJob — yet this another story to write.

Summary

As I said at the beginning, Lombok is a very useful tool, but it depends on us how we will use it. It should help you save your time on stop playing with boilerplate, but it cannot be used to model your domain without a thought.

As a final thought, do not stop playing with modeling your domain. I always have a lot of fun when I’m building API of my domain models and when I can tweak it in each another step. Lombok shouldn’t be used to model your domain, model your domain with Lombok's help.

Side note: do not use Lombok in your library project or setup a build to collect sources of your library using lombok-processed source code. In other case a lot of users will blame you because of Library source does not match the bytecode for class.