447

I've been working with JPA (implementation Hibernate) for some time now and each time I need to create entities I find myself struggling with issues as AccessType, immutable properties, equals/hashCode, ... .
So I decided to try and find out the general best practice for each issue and write this down for personal use.
I would not mind however for anyone to comment on it or to tell me where I'm wrong.

Entity Class

  • implement Serializable

    Reason: The specification says you have to, but some JPA providers do not enforce this. Hibernate as JPA provider does not enforce this, but it can fail somewhere deep in its stomach with ClassCastException, if Serializable has not been implemented.

Constructors

  • create a constructor with all required fields of the entity

    Reason: A constructor should always leave the instance created in a sane state.

  • besides this constructor: have a package private default constructor

    Reason: Default constructor is required to have Hibernate initialize the entity; private is allowed but package private (or public) visibility is required for runtime proxy generation and efficient data retrieval without bytecode instrumentation.

Fields/Properties

  • Use field access in general and property access when needed

    Reason: this is probably the most debatable issue since there are no clear and convincing arguments for one or the other (property access vs field access); however, field access seems to be general favourite because of clearer code, better encapsulation and no need to create setters for immutable fields

  • Omit setters for immutable fields (not required for access type field)

  • properties may be private
    Reason: I once heard that protected is better for (Hibernate) performance but all I can find on the web is: Hibernate can access public, private, and protected accessor methods, as well as public, private and protected fields directly. The choice is up to you and you can match it to fit your application design.

Equals/hashCode

  • Never use a generated id if this id is only set when persisting the entity
  • By preference: use immutable values to form a unique Business Key and use this to test equality
  • if a unique Business Key is not available use a non-transient UUID which is created when the entity is initialised; See this great article for more information.
  • never refer to related entities (ManyToOne); if this entity (like a parent entity) needs to be part of the Business Key then compare the ID's only. Calling getId() on a proxy will not trigger the loading of the entity, as long as you're using property access type.

Example Entity

@Entity
@Table(name = "ROOM")
public class Room implements Serializable {

    private static final long serialVersionUID = 1L;

    @Id
    @GeneratedValue
    @Column(name = "room_id")
    private Integer id;

    @Column(name = "number") 
    private String number; //immutable

    @Column(name = "capacity")
    private Integer capacity;

    @ManyToOne(fetch = FetchType.LAZY, optional = false)
    @JoinColumn(name = "building_id")
    private Building building; //immutable

    Room() {
        // default constructor
    }

    public Room(Building building, String number) {
        // constructor with required field
        notNull(building, "Method called with null parameter (application)");
        notNull(number, "Method called with null parameter (name)");

        this.building = building;
        this.number = number;
    }

    @Override
    public boolean equals(final Object otherObj) {
        if ((otherObj == null) || !(otherObj instanceof Room)) {
            return false;
        }
        // a room can be uniquely identified by it's number and the building it belongs to; normally I would use a UUID in any case but this is just to illustrate the usage of getId()
        final Room other = (Room) otherObj;
        return new EqualsBuilder().append(getNumber(), other.getNumber())
                .append(getBuilding().getId(), other.getBuilding().getId())
                .isEquals();
        //this assumes that Building.id is annotated with @Access(value = AccessType.PROPERTY) 
    }

    public Building getBuilding() {
        return building;
    }


    public Integer getId() {
        return id;
    }

    public String getNumber() {
        return number;
    }

    @Override
    public int hashCode() {
        return new HashCodeBuilder().append(getNumber()).append(getBuilding().getId()).toHashCode();
    }

    public void setCapacity(Integer capacity) {
        this.capacity = capacity;
    }

    //no setters for number, building nor id

}

Other suggestions to add to this list are more than welcome...

UPDATE

Since reading this article I have adapted my way of implementing eq/hC:

  • if an immutable simple business key is available: use that
  • in all other cases: use a uuid
RBz
  • 896
  • 3
  • 17
  • 34
Stijn Geukens
  • 15,454
  • 8
  • 66
  • 101
  • 7
    This is not a question, it is a request for review with a request for a list. Moreover, it's very open ended and vague, or put differently: Whether a JPA entity is perfect depends on what it is going to be used for. Should we list all things an entity might need in all possible uses of an entity? – meriton May 17 '11 at 18:07
  • I know it's not clear cut question for which I apologize. It is not really a request for a list, rather a request for comments/remarks although other suggestions are welcome. Feel free to elaborate on the possible uses of a JPA entity. – Stijn Geukens May 17 '11 at 18:31
  • I would also want fields to be `final` (judging by your omittance of setters, I would guess you do too). – Sridhar Sarnobat Mar 06 '18 at 20:15
  • Would have to try it but I don't think final will work since Hibernate still needs to be able to set the values on those properties. – Stijn Geukens Mar 08 '18 at 04:47
  • 2
    Where does `notNull` come from? – bruno Oct 29 '19 at 16:30

4 Answers4

153

The JPA 2.0 Specification states that:

  • The entity class must have a no-arg constructor. It may have other constructors as well. The no-arg constructor must be public or protected.
  • The entity class must a be top-level class. An enum or interface must not be designated as an entity.
  • The entity class must not be final. No methods or persistent instance variables of the entity class may be final.
  • If an entity instance is to be passed by value as a detached object (e.g., through a remote interface), the entity class must implement the Serializable interface.
  • Both abstract and concrete classes can be entities. Entities may extend non-entity classes as well as entity classes, and non-entity classes may extend entity classes.

The specification contains no requirements about the implementation of equals and hashCode methods for entities, only for primary key classes and map keys as far as I know.

javaguest
  • 399
  • 2
  • 8
Edwin Dalorzo
  • 76,803
  • 25
  • 144
  • 205
  • 14
    True, equals, hashcode, ... are not a JPA requirement but are of course recommended and considered good practice. – Stijn Geukens May 17 '11 at 18:34
  • 6
    @TheStijn Well, unless you plan to compare detached entities for equality, this is probably unnecessary. The entity manager is guaranteed to return the same instance of a given entity every time you ask for it. So, you can do just fine with identity comparisons for managed entities, as far as I understand. Could you please elaborate a bit more about those scenarios in which you would consider this a good practice? – Edwin Dalorzo May 17 '11 at 19:49
  • 2
    I strive to always have a correct implementation of equals/hashCode. Not required for JPA but I consider it a good practice for when entities or added to Sets. You could decide to only implement equals when entities will be added to Sets but do you always know in advance? – Stijn Geukens May 18 '11 at 07:05
  • 11
    @TheStijn The JPA provider will ensure that at any given time there is only one instance of a given entity in the context, therefore even your sets are safe without implementing equals/hascode, provided that you only use managed entities. Implementing these methods for entities is not free of difficulties, for instance, take a look at this [Hibernate Article](http://community.jboss.org/wiki/EqualsAndHashCode) about the subject. My point being, if you only work with managed entities, you are better off without them, otherwise provide a very careful implementation. – Edwin Dalorzo May 18 '11 at 11:56
  • ...but the standard does not adress this issue. – Edwin Dalorzo May 18 '11 at 12:04
  • The article you mention also seems to recommend implementing eq/hC for JPA entities. It indeed states that it is not required as long as your entities are not detached but can you (as the entity developer) know in advance how your entities will be used (by other developers). It seems safer to me to always have a business key dependant eq/hC. – Stijn Geukens May 18 '11 at 12:16
  • @TheStijn I guess it depends on how you desing you API. For instance, I am working in an API that uses transfer objects as the data communication channel between the persistence layer and the clients. These DTOs are used to create the business objects at the server-side which are the actual entities. So entities never abandon the safety of the persistence layer: [Transfer Object Pattern](http://java.sun.com/blueprints/corej2eepatterns/Patterns/TransferObject.html). Anyway, good discussion. I think it is clear this not required by the standard, but in certain cases is good idea to do it. – Edwin Dalorzo May 18 '11 at 12:47
  • +1 Actually, I'm already using DTOs in the presentation layer but this had become a logical step after implementing DDD. I had not yet considered that, as you say, DTOs keeps your entities in the persistence layer and therefore the eq/hC is not an issue. Unfortunately, not all my fellow developers (big team) follow this approach. – Stijn Geukens May 18 '11 at 13:51
  • 2
    @TheStijn This is the good mixed scenario. It justifies the need of implementing eq/hC as you initially suggested because once the entities abandon the safety of the persistence layer you can no longer trust the rules enforced by the JPA standard. In our case, the DTO pattern was architecturally enforced from the beginning. By design our persistence API does not offer a public way to interact with the business objects, only an API to interact with our persistence layer using DTOs. – Edwin Dalorzo May 18 '11 at 15:11
  • @EdwinDalorzo What about the other constructors? Do we need them for lazy-loading? –  Dec 10 '21 at 06:11
75

I'll try to answer several key points: this is from long Hibernate/ persistence experience including several major applications.

Entity Class: implement Serializable?

Keys needs to implement Serializable. Stuff that's going to go in the HttpSession, or be sent over the wire by RPC/Java EE, needs to implement Serializable. Other stuff: not so much. Spend your time on what's important.

Constructors: create a constructor with all required fields of the entity?

Constructor(s) for application logic, should have only a few critical "foreign key" or "type/kind" fields which will always be known when creating the entity. The rest should be set by calling the setter methods -- that's what they're for.

Avoid putting too many fields into constructors. Constructors should be convenient, and give basic sanity to the object. Name, Type and/or Parents are all typically useful.

OTOH if application rules (today) require a Customer to have an Address, leave that to a setter. That is an example of a "weak rule". Maybe next week, you want to create a Customer object before going to the Enter Details screen? Don't trip yourself up, leave possibility for unknown, incomplete or "partially entered" data.

Constructors: also, package private default constructor?

Yes, but use 'protected' rather than package private. Subclassing stuff is a real pain when the necessary internals are not visible.

Fields/Properties

Use 'property' field access for Hibernate, and from outside the instance. Within the instance, use the fields directly. Reason: allows standard reflection, the simplest & most basic method for Hibernate, to work.

As for fields 'immutable' to the application -- Hibernate still needs to be able to load these. You could try making these methods 'private', and/or put an annotation on them, to prevent application code making unwanted access.

Note: when writing an equals() function, use getters for values on the 'other' instance! Otherwise, you'll hit uninitialized/ empty fields on proxy instances.

Protected is better for (Hibernate) performance?

Unlikely.

Equals/HashCode?

This is relevant to working with entities, before they've been saved -- which is a thorny issue. Hashing/comparing on immutable values? In most business applications, there aren't any.

A customer can change address, change the name of their business, etc etc -- not common, but it happens. Corrections also need to be possible to make, when the data was not entered correctly.

The few things that are normally kept immutable, are Parenting and perhaps Type/Kind -- normally the user recreates the record, rather than changing these. But these do not uniquely identify the entity!

So, long and short, the claimed "immutable" data isn't really. Primary Key/ ID fields are generated for the precise purpose, of providing such guaranteed stability & immutability.

You need to plan & consider your need for comparison & hashing & request-processing work phases when A) working with "changed/ bound data" from the UI if you compare/hash on "infrequently changed fields", or B) working with "unsaved data", if you compare/hash on ID.

Equals/HashCode -- if a unique Business Key is not available, use a non-transient UUID which is created when the entity is initialized

Yes, this is a good strategy when required. Be aware that UUIDs are not free, performance-wise though -- and clustering complicates things.

Equals/HashCode -- never refer to related entities

"If related entity (like a parent entity) needs to be part of the Business Key then add a non insertable, non updatable field to store the parent id (with the same name as the ManytoOne JoinColumn) and use this id in the equality check"

Sounds like good advice.

Hope this helps!

Arjan Tijms
  • 37,782
  • 12
  • 108
  • 140
Thomas W
  • 13,940
  • 4
  • 58
  • 76
  • 3
    Re: constructors, I often see zero arg only (i.e. none) and the calling code has a great long list of setters which seems a bit messy to me. Is there really any issue with having a couple of constructors which suit your need, making the calling code more succinct? – Hurricane Nov 11 '16 at 00:26
  • totally opinionated, specially about ctor. what is more beautiful code? a bunch of different ctors that lets you know which (combination of) values are needed to create an sane state of the obj or a no-arg ctor which gives no clue what shall be set and in which order and leaves it prone to user's mistakes? – mohamnag Feb 02 '18 at 10:35
  • 1
    @mohamnag Depends. For internal system-generated data strictly valid beans are great; however modern business applications consist of large numbers of user data-entry CRUD or wizard screens. User-entered data is often partly or poorly-formed, at least during editing. Quite often there is even business value in being able to record an incomplete state for later completion -- think insurance application capture, customer signups etc. Keeping constraints to a minimum (eg. primary key, business key & state) allows greater flexibility in real business situations. – Thomas W Feb 07 '18 at 03:33
  • 1
    @ThomasW first I have to say, I'm strongly opinionated towards domain driven design and using names for class names and meaning full verbs for methods. In this paradigm, what you are referring to, are actually DTOs and not the domain entities that should be used for temporary data storage. Or you have just misunderstood/-structured your domain. – mohamnag Feb 07 '18 at 12:50
  • @ThomasW when I filter out all the sentences your trying to say I'm a novice, there is no information in your comment left except regarding user input. That part, as I said before, shall be done in DTOs and not in entity directly. lets talk in another 50 years that you may become 5% of what big mind behind DDD like Fowler has experienced! cheers :D – mohamnag Feb 08 '18 at 15:43
  • @mohamng Well, I pointed out a divide in business scenarios, as well some principles on operational completeness -- but you didn't seem to understand those points. The scenarios I was talking about -- eg. insurance quotes or applications -- actually involve persistent partial data in early states of the application lifecycle, they're not just UI editing or DTOs. – Thomas W Feb 14 '18 at 02:52
  • This question is also asking about Hibernate entities; persistence is naturally cross-cutting to business domain behavior. This answer dates to Hibernate 3/4, but from Hibernate's origins standard reflection & a no-arg constructor have been the simplest & most basic method by which Hibernate can load/ store data. – Thomas W Feb 14 '18 at 08:04
  • Invariants should be protected agressively. If today all customers needs an address it makes sense to enforce such rule in the ctor. If the rule changes then the model simply must evolve with it. The same goes for the insurance quote example. If it's valid for some fields to be empty while drafting the quote then those being null is a valid state. Perhaps those could be mandatory to transition to some status though: the invariant being must not be null in status S. – plalx Nov 25 '20 at 04:46
  • Thanks @plalx. My philosophy here is informed by several factors: 1) Hibernate typically creates via a no-arg constructor, 2) partial states can often be useful for data capture or operational reasons, and 3) I would distinguish between universal truths versus this year's business rules. I'm aware much dogma exists around constructors enforcing business rules, but have seen problems with this on many occasions. I would only model rules around genuine (PK or business) keys. Address is not such a field, and most businesses would find a way to take the customer's money without. – Thomas W Nov 27 '20 at 03:10
  • "would distinguish between universal truths versus this year's business rules". I'm just not sure to understand what's the problem to prevent a state representation that's currently invalid and loosen up that rule later when the rule becomes too strict for new business scenarios? Could you expand on some examples where it was problematic to take an always-valid approach for invariants? – plalx Nov 27 '20 at 03:34
15

My 2 cents addition to the answers here are:

  1. With reference to Field or Property access (away from performance considerations) both are legitimately accessed by means of getters and setters, thus, my model logic can set/get them in the same manner. The difference comes to play when the persistence runtime provider (Hibernate, EclipseLink or else) needs to persist/set some record in Table A which has a foreign key referring to some column in Table B. In case of a Property access type, the persistence runtime system uses my coded setter method to assign the cell in Table B column a new value. In case of a Field access type, the persistence runtime system sets the cell in Table B column directly. This difference is not of importance in the context of a uni-directional relationship, yet it is a MUST to use my own coded setter method (Property access type) for a bi-directional relationship provided the setter method is well designed to account for consistency. Consistency is a critical issue for bi-directional relationships refer to this link for a simple example for a well-designed setter.

  2. With reference to Equals/hashCode: It is impossible to use the Eclipse nor Lombok auto-generated Equals/hashCode methods for entities participating in a bi-directional relationship, otherwise they will have a circular reference resulting in a stackoverflow Exception. Once you try a bidirectional relationship (say OneToOne) and auto-generate Equals() or hashCode() or even toString() you will get caught in this stackoverflow exception.

Sym-Sym
  • 3,578
  • 1
  • 29
  • 30
10

Entity interface

public interface Entity<I> extends Serializable {

/**
 * @return entity identity
 */
I getId();

/**
 * @return HashCode of entity identity
 */
int identityHashCode();

/**
 * @param other
 *            Other entity
 * @return true if identities of entities are equal
 */
boolean identityEquals(Entity<?> other);
}

Basic implementation for all Entities, simplifies Equals/Hashcode implementations:

public abstract class AbstractEntity<I> implements Entity<I> {

@Override
public final boolean identityEquals(Entity<?> other) {
    if (getId() == null) {
        return false;
    }
    return getId().equals(other.getId());
}

@Override
public final int identityHashCode() {
    return new HashCodeBuilder().append(this.getId()).toHashCode();
}

@Override
public final int hashCode() {
    return identityHashCode();
}

@Override
public final boolean equals(final Object o) {
    if (this == o) {
        return true;
    }
    if ((o == null) || (getClass() != o.getClass())) {
        return false;
    }

    return identityEquals((Entity<?>) o);
}

@Override
public String toString() {
    return getClass().getSimpleName() + ": " + identity();
    // OR 
    // return ReflectionToStringBuilder.reflectionToString(this, ToStringStyle.MULTI_LINE_STYLE);
}
}

Room Entity impl:

@Entity
@Table(name = "ROOM")
public class Room extends AbstractEntity<Integer> {

private static final long serialVersionUID = 1L;

@Id
@GeneratedValue(strategy = GenerationType.AUTO)
@Column(name = "room_id")
private Integer id;

@Column(name = "number") 
private String number; //immutable

@Column(name = "capacity")
private Integer capacity;

@ManyToOne(fetch = FetchType.LAZY, optional = false)
@JoinColumn(name = "building_id")
private Building building; //immutable

Room() {
    // default constructor
}

public Room(Building building, String number) {
    // constructor with required field
    notNull(building, "Method called with null parameter (application)");
    notNull(number, "Method called with null parameter (name)");

    this.building = building;
    this.number = number;
}

public Integer getId(){
    return id;
}

public Building getBuilding() {
    return building;
}

public String getNumber() {
    return number;
}


public void setCapacity(Integer capacity) {
    this.capacity = capacity;
}

//no setters for number, building nor id
}

I don't see a point of comparing equality of entities based on business fields in every case of JPA Entities. That might be more of a case if these JPA entities are thought of as Domain-Driven ValueObjects, instead of Domain-Driven Entities (which these code examples are for).

ahaaman
  • 552
  • 5
  • 22
  • 4
    Although it is a good approach to use a parent entity class to take out the boiler plate code it's not a good idea to use the DB defined id in your equals method. In your case comparing 2 new entities would even throw a NPE. Even if you make it null safe then 2 new entities would always be equal, until they are persisted. Eq/hC should be immutable. – Stijn Geukens Jun 03 '13 at 14:19
  • 2
    Equals() won't throw NPE as there is check whether DB id is null or not & in case DB id is null, the equality would be false. – ahaaman Aug 01 '13 at 09:53
  • 3
    Indeed, I don't see how I missed that the code is null-safe. But IMO using the id is still bad practice. Arguments: http://www.onjava.com/pub/a/onjava/2006/09/13/dont-let-hibernate-steal-your-identity.html – Stijn Geukens Aug 01 '13 at 15:31
  • In the book 'Implementing DDD' by Vaughn Vernon, it is argued that you can use id for equals if you use "early PK generation" (Generate an id first and pass it into the constructor of the entity as opposed to letting the database generate the id when you persist the entity.) – Wim Deblauwe Dec 09 '15 at 09:36
  • or if you do not plan on equal compare non persistent entities? Why should you... – Enerccio Dec 26 '17 at 07:26