Thoughts from the team at Mission Data

Hibernate and your Getters and Setters

When you’re using Hibernate and are mapping to properties, keep your getters and setters as simple and self-contained as possible. The receiver being initialized may not have any other properties set, and the value being passed may not be fully initialized yet, either.

If you don’t respect these two possibilities, then you will get bit in the butt alot, when you least expect it. To be fair, these situations can happen whether you’re using Hibernate or not, but when we first started using the framework we made lots of assumptions.

Here is a completely ridiculous example that violates the above restrictions:

public class User
{
  private boolean isSurnameLast;
  private Name name;  

  private String fullName;
  private String initials;

  public boolean setSurnameLast(boolean surnameLast)
  {
    this.isSurnameLast=surnameLast;
  }

  public void setName(Name name)
  {
    this.name = name;
    String lastName;
    String firstName;
    initials = new StringBuffer(name.getSurName().substring(0,1)).append(".").append(name.getGivenName().substring(0,1)).toString();
    if(isSurnameLast)
    {
      lastName = name.getSurName();
      firstName = name.getGivenName();
    }
    else
    {
      lastName = name.getGivenName();
      firstName = name.getSurName();
    }
    fullName = new StringBuffer(firstName).append(" ").append(lastName).toString();
  }
  // more getters and setters blah blah blah
}

(Ignore the lame “business logic” here…)

So you look in your database and you see that all the Users database entries have names and properly set surnameLast values, and all the Names database entries have a surName and a givenName. Sure the code is sloppy, but this is gonna work fine!

Nope, this setName is loaded with problems.

First, you’ll notice as your app runs that somehow setName is being called and passed a null. What?! Why! Durn that Hibernate, it is trying to trick you! No matter, we’ll catch it with a null check. You’ll shield your eyes as to when and why Hibernate or cglib or whatever is calling setters on an instance that isn’t even in the database, but at least it won’t blow up. This is the first bad smell you’ve blown through, you sloppy panicky codemonkey:

  public void setName(Name name)
  {
    if(name!=null) // hahah
   {
       this.name = name;
      ....

Then you’ll start realizing that no matter what the database says, sometimes you are getting the surname first in the full name! “Smith Joan!”, you’ll cry, but “it’s true for Joan Smith in the database! Why?” Well, sometimes Hibernate is calling setName before it calls setSurnameLast. It doesn’t matter which you have mapped first in the User.hbm.xml file, or which field you have first in the code. You can’t depend on other properties of User being initialized in the Hibernate setters.

Now, you’re also going to get NullPointerExceptions thrown out of this code, because sometimes that Name instance you’re being passed hasn’t had its properties initialized by Hibernate yet. Talk about something that seems sneaky and evil but is totally above board: Hibernate has created the new Name instance and has passed that reference to other instances, but hasn’t fully populated the Name instance yet.

That means this line will blow sky-high sometimes:

initials = new StringBuffer(name.getSurName().substring(0,1)).append(".").append(name.getGivenName().substring(0,1)).toString();

The sane thing to do here?

  public void setName(Name name)
  {
    this.name = name;
  }

So if you’re using Hibernate, go take a look at your classes and mappings! You may very well be doing something dangerous.

3 Responses to “Hibernate and your Getters and Setters”

  1. March 11th, 2007 @ 7:28 pm Rich Rodriguez responded:

    I’m pretty satisfied using access=”field” with Hibernate. Yes, it breaks encapsulation, but it keeps you from running into these maddening bugs. I blogged a while back about how setter logic was defeating the batch load functionality in Hibernate.

  2. March 11th, 2007 @ 9:17 pm at Obscured Info responded:

    […] Over at the MD blog Darren has illustrated more dangers with Hibernate setters. […]

  3. February 19th, 2008 @ 4:23 pm Sami Dalouche responded:

    sorry, I repeat

    If you consider the DDD (Domain Driven Design) approach a good one, then
    I don’t believe that breaking encapsulation really is an issue.

    Indeed, if you consider that your entities constitute a rich domain, then you might not even want setters, you might want a fluent interface to set your parameters, etc..

    So, Entities can be created by factories and persisted using Repositories. I think it is way better to have stronger encapsulation on the domain and break it in the factories and repositories, instead of having a weaker encapsulation that is not broken anywhere.

    So, in this direction, direct field access doesn’t seem stupid

blog comments powered by Disqus