0

I have inherited a class with the following Lombok annotations:

@Getter
@Setter
@Builder
@NoArgsConstructor
@AllArgsConstructor
@ToString
@EqualsAndHashCode
public class PricingAndCosting {

//variables and constants

}

is it better practice to just do the following to get the same functionality:

@Entity
@Data
public class PricingAndCosting {

//variables and constants

}
java123999
  • 6,974
  • 36
  • 77
  • 121
  • `@Data` does not have `@Builder`, have a look: https://projectlombok.org/features/all – dkb May 15 '19 at 08:54
  • 1
    `@Data` does not generate `@NoArgsConstructor` or `@AllArgsConstructor` ( but `@RequiredArgsConstructor`), so, you would be missing those two previously mentioned. Neither it includes `@Builder` as mentioned by dkb. – lealceldeiro May 15 '19 at 08:54

2 Answers2

1

The class you had inherited is only 1 step removed from being fully open to the outside (it would require making the fields public) which is potentially bad as few bugs could hide in uses of this class. Even if the class is being consumed in sane way, you cannot guarantee it into the future unless you close it off a bit.

You can construct it with no values, with all values, you can build it with only some values and you can mutate it through setters.

This most likely breaks the equals and hashcode contract too (those should be calculated using the same set of immutable fields - a very rough rule of thumb which makes sense when using lombok, but obviously there is more).

I would first identify the uses of this class and find out whether it can be refactored to something closer to immutable.

diginoise
  • 7,352
  • 2
  • 31
  • 39
  • "This most likely breaks the equals and hashcode contract too" What exactly do you mean by this? Provided that `null` is a valid value for the field, why shouldn't `new Obj(1, null)` and `new Obj(1, null)` be equal? Or for that matter, `new Obj(null, null)` and `new Obj(null, null)`? – filpa May 15 '19 at 12:43
  • 2
    `var dan = new User("dan"); userList.put(dan); dan.setName("Daniel"); userList.contains(dan) == false;` See this question too: https://stackoverflow.com/questions/1076652/mutable-fields-for-objects-in-a-java-set – diginoise May 15 '19 at 13:34
  • Huh. You learn something every day. For anyone wondering, this does result in very unexpected behaviour, see [this](https://gist.github.com/filpano/306556360f86b20085c8ab04809c0984) gist. However, I would see this more as breaking the `Set` contract rather than the `equals` and `hashCode` contracts, since the generated Lombok functions would generate the hashcode using the new field value after mutation. – filpa May 15 '19 at 14:01
-1

Short answer is it depends. Long answer is in general, Lombok is generating code for you to avoid boilerplate code, however, you should know what you really need to use? suppose you need only getters then use @Getter.

Moreover, sometimes it is really helpful for you; something which is common heavily uses like Builder pattern can be offered to you with just one annotation, and also Throwing an exception is another fancy feature and more and more. You can check this to see a good example https://www.baeldung.com/intro-to-project-lombok

As code efficiency; it sounds that using little code with @Data will be better, but actually, it is not! you are consuming resources which you may not need. So try to use what you really need is a good practice.

Mohamed Sweelam
  • 1,109
  • 8
  • 22