views:

390

answers:

3

One challenge using hibernate is that manged classes have to have a default constructor. The problem is that there is no explicit point where the class is initialized and invariants can be checked.

If a class has invariants that depend on more than one property the class design gets complex. Let's start with the hypothetical green-field design:

public class A { 
    private int x; 
    private int y; 

    public A(int x, int y) { 
        this.x = x; 
        this.y = y; 
        checkInvariants(this.x, this.y); 
    } 

    private void checkInvariants(int x, int y) { 
        if (x + y « 0) throw new IllegalArgumentException(); 
    } 
}

This is the base implementation that does not meet the hibernate requirements. The invariant is checked in the constructor. (The content of the checkInvariants() method does not matter it's only presented to illustrate that the class invariants can depend on more that one property.)

The class can be used as follows:

new A(0, 0); 
new A(-1, 0); //invalid

To meet the hibernate requirements one workaround is to add a private default constructor and use field access. (I omitted the hibernate mappings.)

public class H { 
    int x; 
    int y; 

    public H(int x, int y) { 
        this.x = x; 
        this.y = y; 
        checkInvariants(this.x, this.y); 
    } 

    H(){} 

    private void checkInvariants(int x, int y) { 
        if (x + y « 0) throw new IllegalArgumentException(); 
    } 
}

This has two major drawbacks: * You're starting to implement code that depends on the client (Hibernate). Ideally, a class does not know its callers. * A specific issue with this workaround is that instances initiated by hibernate are not checked if the meet the invariants. You're trusting data that is loaded from the database which is problematic. Even if your application is the only one using this specific database schema there is always the possibility of ad-hoc changes by administrators.

A second workaround is to check invariants in user code:

public class I { 
    private int x; 
    private int y; 

    public I() {} 

    public void checkInvariants() { 
        if (x + y « 0) throw new IllegalArgumentException(); 
    } 

    public void setX(int x){ 
        this.x = x; 
    } 

    public void setY(int y){ 
        this.y = y; 
    } 
} 

I i = new I(); 
i.setX(-1); 
i.setY(0); 
i.checkInvariants();

Obviously, this makes the user code more complex and error-prone. This design does not fulfill the expectation that an instance is consistent after creation and stays consistent after each change of state (method call). Every user has to check the invariants for every instance he creates (maybe indirectly with hibernate).

Is there a better solution to this problem which is:

  • not overly complex
  • without explicit knowledge about its users
  • without a dependency to the hibernate framework?

I suppose some of the constraints have to be loosened to get to a pragmatic solution. The only hard constraint is that there is no dependency to the hibernate framework. (Hibernate specific code outside of domain objects is okay).

(Just out of curiosity: is there a ORM framework that does support “constructor injection”?)

+1  A: 

One approach would be based on your "class I", but with the class itself calling checkInvariants() the first time the fields are actually used. This allows for the fields to be temporarily invalid during assignment (after setX() is called but before setY() for example). But it still assures that only valid data is ever used.

Your example class never uses the values, so I'll presume values could be accessed via getX(), getY(), and doSomething(), such as:

public int getX(){ 
    return x; 
} 

public int getY(){ 
    return y; 
} 

public void doSomething(){
    x = x + y;
}

You'd change it like this:

private boolean validated = false;

public int getX(){ 
    if (!validated) {
        checkInvariants();
    }

    return x; 
} 

public int getY(){ 
    if (!validated) {
        checkInvariants();
    }
    return y; 
} 

public void doSomething(){
    if (!validated) {
        checkInvariants();
    }
    x = x + y;
}

public void checkInvariants() { 
    validated = true;
    if (x + y « 0) throw new IllegalArgumentException(); 
}

You could move the 'if (!validated)' into checkInvariants() if desired.

You might also look into the Hibernate Validator, though it may not meet your framework-independence requirement. http://docs.huihoo.com/hibernate/annotations-reference-3.2.1/validator.html

D Adler
+2  A: 

First, let me address the "drawbacks" you've listed to your first approach:

You're starting to implement code that depends on the client (Hibernate). Ideally, a class does not know its callers.

You're using the word "dependency" a bit fast and loose here. Hibernate is not a "client"; it's a framework you (as developer / architect / what have you) chose to implement your persistence with. Therefore, you're going to have some code somewhere that uses (and, thus, depends on) Hibernate. That said, there is NO dependency on Hibernate in your domain object above. Having a no-arg constructor is a semantic requirement if you will; it does not introduce an actual dependency. Switch Hibernate for JPA / TopLink / raw jdbc / what have you and you won't have to change a single thing in your domain object code.

A specific issue with this workaround is that instances initiated by hibernate are not checked if the meet the invariants. You're trusting data that is loaded from the database which is problematic. Even if your application is the only one using this specific database schema there is always the possibility of ad-hoc changes by administrators.

You don't have to "trust" the data (more on that below). However, I don't think this argument has merit. If you're modifying your data in multiple applications, you should be performing validations at some common lower layer instead of relying on each individual application to validate the data. Said common layer could be the database itself (in simple cases) or a service layer providing common API to be used by your multiple applications.

Furthermore, the notion of administrators making changes directly to the database as part of daily routine is utterly ridiculous. If you're talking about special cases (bug fixes, what have you) they should be treated as such (that is, presumably something like this happens very rarely and the burden to validate such "critical" changes lies on whoever makes the changes; not on each of your applications in the stack).

All that said, if you do want to validate your object when they're loaded, it's reasonably straightforward to achieve. Define a Valid interface that has validate() method and have every concerned domain object implement it. You can invoke that method from:

  1. Your DAO / service after the object has been loaded.
  2. Hibernate Interceptor or Listener - both are set up in Hibernate configuration; all you need to do is to implement either one to check whether the object being loaded implements Valid and, if so, invoke the method.
  3. Or you can use Hibernate Validator, however that WILL tie your domain objects to Hibernate as you'd be annotating them.

Finally, as far as "constructor injection" goes - I do not know of any framework that supports it directly. The reason for this is quite simple - it only makes sense for immutable entities (because once you have setters you have to deal with validation anyway) and thus means a lot of work (handling constructor parameter mappings, etc...) for almost zero net effect. In fact, if you're that concerned with having a no-arg constructor for immutable objects you can always not map them as entities and instead load them via HQL which does support constructor injection:

select new A(x, y) from ...

Update (to address points from Thomas's comments):

  1. I've only mentioned the interceptor for completeness; listener is much more appropriate in this case. PostLoadEventListener is called after entity is fully initialized.
  2. Once again, having no-arg constructor is not a dependency. It's a contract, yes, but it it doesn't tie your code to Hibernate in any way. And as far as contracts go, it's a part of javabean spec (in fact, it's less restrictive because constructor doesn't have to be public) so in majority of cases you would follow it anyway.
  3. Accessing the database. "Database refactoring and migrations are common" - yes, they are. But it's just code; you write it, you test it, you run the integration tests, you deploy it to production. If your domain model object uses some StringUtil.compare() method you wrote in some utility class, you don't have it recheck the results, do you? Similarly, domain object should not have to check that your migration script didn't break anything - you should have appropriate tests for that. "Ability to do ad-hoc queries ... is one of the features" - absolutely. Queries. As in "read-only" queries used for reporting, for example (even so, in many cases it's more appropriate to go through the API). But manual data manipulation as non-emergency - absolutely not.
  4. Mutability makes constructor injection irrelevant. I'm not saying you can't have non-default constructor - you can, and you can use it in your code. But if you have setter methods you can not use your constructor for validation, so whether it is or isn't there doesn't really matter.
  5. HQL constructor injection and associations. Nested constructors won't work as far as I know (e.g. you can't write select new A(x, y, new B(c, d)); so in order to fetch associations you're going to need to retrieve them as entities in select clause which means they need to have no-arg constructors themselves :-) Or you can have a constructor on "main" entity that takes all needed nested properties as arguments and constructs/populates associations internally but that's borderline crazy :-)
ChssPly76
The combination of hibernate interceptors and a validation interface is a nice solution. The user code can be left unchanged using the constructor A(x,y) and the private constructor + field access is + validation is accessed by hibernate. (Implementing the interceptor will not be that easy. The entity is not initialized when onLoad is called.)
Thomas Jung
Dependency: Your domain code is not independent from the ORM framework. There is an underlining contract you have to fulfill to work with the framework and that's what you depend on. (It does not matter that the contract may be the same for different frameworks. It is still there.)
Thomas Jung
Accessing the database directly: Once is enough. Your application is inconsistent with all the consequences. So it is my job to check that all data is consistent (otherwise the business would be duplicated in your database scripts). Database refactoring and migrations are common and not a corner case. The ability to do ad-hoc queries against a relational database is one of the features why relational databases are still used.
Thomas Jung
Constructor injection: I do not see why objects whith an non-default constructor that are mutable do not take advantage from the constructor injection. Your client code is: A(1,1); against: A(); A.setX(1); A.setY(1); that is clearly better. A lot of work once is better than a lot of suboptimal user code in a lot of places.
Thomas Jung
The HQL constructor injection won't work with associations (lazy association loading), I suppose?
Thomas Jung
@Thomas - I've updated my answer above to reply to your points.
ChssPly76
Update: 1. Ok 2. We should skip that. I do see design constraints as a dependency you don't. I think that is not a difference in practice at the implementation level. 3. Double checking is a life assurance not a problem (we have type systems and assertions and tests and reviews ... and there are still bugs). 4. I still want my users to write A(1,1) instead of A();A.set(1);A.set(1);. 5. Ok
Thomas Jung
+3  A: 

The Coherence of the relational model is critically important concept to gasp. Because of the inherent mathematical stability of the principles that underlie relation data modeling, we can be totally confident that the result of any query of our original database will indeed generate qually valid facts. - From the book The Art of Sql - Stephane Faroult

Your database should consist of valid facts or truths, data that you load from the database should not require any extra validations, the fact that you pulled it out from the database should be good enough.

But if you are concerned that there is bad data, then I would suggest that there is a more serious problem.

I have seen solutions ranging from having a staging database where all the data is scrubbed , validated before it enters the real database to having reviews of the any manual fix before it is entered.

Either way, once you have corrupted data or false statements in your database, what else can you trust ?

Devender Gollapally
Sorry if I got preachy
Devender Gollapally
Amen to that (and +1)
ChssPly76