Don't use entities as value objects

[monrealis]

@Entity (identified by unique field) and @Data (value object) annotations on the same class? Entity is the opposite of value object, at least according to my experience and DDD. Doesn't this example encourage bad practice?

See https://github.com/spring-projects/spring-data-examples/blob/bee8a81118e1cf3fec3ef5ac91c4e9d3677b6a74/jpa/eclipselink/src/main/java/example/springdata/jpa/eclipselink/Customer.java and other classes. If a female customer is put to HashSet, then gets married and changes her lastname, wouldn't you expect to find her in that HashSet?

[gregturn]

@Data simply means create getters, setters, toString, hashCode, and equals methods, automatically based on the fields. It's by no means an immutable value object.

To actually create something that's immutable, you would use Lombok's @Value. Since JPA doesn't like immutability, this doesn't work very well.

[monrealis]

The problem is that equality and hash code is based on all fields, not on id alone. But it should be based on id if it is filled, as you can see from my example. There's no difference if it is so because of hand coded, inherited, or generated equals() and hascCode() methods.

[gregturn]

I guess it depends on which comparison you are making. When holding two Customer objects and comparing them side by side, are we assuming a match based on them being the same row in the table, or based on the value of their contents? If purely based on id, then we could be viewing the same row of data from two different times where their values have changed. I wouldn't call that equal.

As for mutating members of Set, that sounds risky on the surface. But to go further, any "view" of the data can be deemed risky because if the person's surname (or any attribute) was updated in the database, my copy is stale.

This whole topic begs the usage of https://github.com/spring-projects/spring-data-envers to deploy versioning of your data if the management of such changes becomes complex. (BTW, Lombok's @Data annotation DOES let you specify exactly which fields apply in hashCode/equals, if you wish).

But if we're talking about storing a handful of objects for a demo to illustrate concepts of Spring Data, with little input on the rate of change requirements, then I don't see the point of worrying about it right now.

[monrealis]

There are good books regarding this topic: "Domain-driven design", "Patterns of Enterprise Application Architecture", .... I wonder if you could name a single book that would recommend that a class similar to Customer included all fields in its equality check. By the way, https://martinfowler.com/bliki/EvansClassification.html has a Customer example.

According to DDD, entities are defined by a thread of continuity and identity. You would normally want to know if an order came from the same customer, even though some data in that customer has changed. If a customer revision 1 spent 100 and that same customer revision 2 spent 1000, you would want to know that that customer spent 1100 in total, despite someone has updated one of his 100 fields for any reason (possibly event without a business reason). Database ID is a good candidate to identify and entity, because it never changes. If there were other fields, that identify entity and do no change over time, they would be valid for equality check (even more valid than ID, since ID is usually generated only when persisting object to database). But using ID is much easier, you don't have to think much, and you can uniformly apply the pattern. If there's no database, it's hard, you have to invent unique fields in every class.

By the way, maybe a customer has some collection of objects, and each of their objects have their own collections. In such a case you would be comparing whole graphs of objects, possibly with cycles in references. And by the way, hash code has to match equals logic too, you would have to walk that graph to get the hash code. If there's a cycle, you are in trouble (check this https://gist.github.com/monrealis/3763f909eeeef34df27b82f6e27a25a1).

Even if @Data allows the fields to be specified, they weren't specified. It would be OK, if ID were specified, but it isn't.

Thanks for the link to "Spring Data Envers", I will look at it, but it is not related to this issue.

[gregturn]

I'm quite familiar with DDD. In fact, project lead @olivergierke has given many talks on DDD & REST.

Again, for the scope of the code at hand, I'm not sure I see the necessity to split apart Customer the entity from Customer the value object. For more complex scenarios, I get it. But I don't see the need here. Thanks.

[odrotbohm]

To add on what Greg said: yes you're right, the equals(…) and hashCode() methods should be based on the identifier and we're happy to accept a pull request for that.

That said, the example is by no means about DDD but on how to set up Spring Data JPA with Eclipselink, i.e. the focus is not on getting the domain right (as there is none) but the configuration code. That's why we think there's more important things to worry about. Example naturally set some focus point and don't try to get everything right, as additional code that's needed might distract from the actual aspect the example is supposed to show.

[odrotbohm]

Nevermind the PR, I just pushed the fix.

[monrealis]

Thanks! We will avoid the risk that someone copies the example to their own project without considering too much what equals() and hashCode() gets generated with lombok @Data annotation.

[monrealis]

A small update: there's a course on Pluralsight called "Domain-Driven Design Fundamentals" and there's a chapter called "Eric Evans on the entity equality". He does not like equals() methods in entities at all. But the demo goes on with equals() based on ID for entities (in the next chapter). By the way, class under consideration is surprise surprise Customer (the most popular concept in programming?).

[odrotbohm]

You don't have to convince anyone. It's just that in the context of an example we want to write as little distracting code from what the example is about as possible. Because if you want to implement everything properly (transactions, security) you end up with the actual thing to show burried in a plethora of lines of code of related aspects. The Eclipselink example is purely about the setup, the configuration code. Everything else is just accessory.