I'm starting to get some rather stinky code in one of my projects and wanted to ask for some advice. (I'm not sure if this should be in Struts/Testing/or refactoring... but I think it belongs here as the solution probably involves knowing more about how Struts 2 operates)
Its a Struts2/Spring2/JPA(Hibernate) based project. I'm using a slightly modified version of the Generic DAO pattern shown in the Java persistence with Hibernate book and/or the IBM ThoughtWorks very similar example. (Modified to allow Spring Based Injection of the JPA EntityManger, while falling back to a native Hibernate session inside the DAOs to allow a few more optimizations).
So its basically Business Objects (POJOs) <---1:1---> DAOs which is a relatively normal pattern I beleive.
Now I'm working on an Action that creates an object that contains lots of references to other objects. A bug tracker would be a reasonable facsimile of my domain object -- when creating a bug, you select a severity, a project, a module, a found in version, possibly you're assigning it to someone, etc. Most of these options come from a drop-down on the view page, however they are all backed by their own domain object -- ie its not just id/string display name lookup-table backing the drop down.
So the form submits a whole suite of IDs or String natural keys and I need to build an Defect/Bug and then persist it. In building the Defect I need to convert all the keys to domain objects. This is where things begin to fall apart. To covert the keys to domain object seems to require injecting all the subordinate object DAOs into the Action, its not a big deal but its starting to feel like the subordinate DAOs are taking over the action. Furthermore, its slightly annoying from a performance stand-point that I need to retrieve all the objects from the DB, just to set a foreign key -- I will never be updating the details on the subordinate object from the master only changing which one I'm linked to. This is a place where the Get (returning a proxy/lazy load thunk with only the ID set without hitting the DB) versus Load could be useful, but I've never seen any of the generic DAO approaches expose that level of control in their API -- does anyone know why?
Here's an example (not from live code, please ignore any minor typos) representing the current state of application and test code. Afterwards I'll list a few of the approaches I've considered for cleaning it up: AddBug.java
TestAddBug.java (Sorry this is purely from memory so I know I'm missing some of my infrastructure)
OK. Now that you've seen the mess... I've thought of two ways to clean it up:
1) Introduce a business tier between the domain objects and the DAOs. Expose a buildBug call in this layer that takes in the keys and handles the promotion to subordinate domain objects. This business tier would need to have all the DAO injected into it. Actions would still have their most relevant DAO(s) injects, plus the business tier relevant objects. Testing the business tier would still require the mess of mocks, but at least the action only needs to mock the single call on the business tier. However I don't think this approach would play too well with migrating to a Model-Driven strategy in the future as the domain objects can't accept the bare keys. I'm also having a hard time figuring out what type of methods would end up in this layer -- it seems like mostly it will be these kinds of builders... Most of cases are either correctly cascaded/handled by hibernate and the DAOs or they belong on the POJOs... Leading to a very anemic layer, which doesn't seem worth the added complexity?
2) Setup a series of type converters to promote the keys to objects on the incoming HTTP request end of things. The type converters would need access to the DAOs. Testing them should be rather simple as each singular one is simple. Testing the action no longer requires mocking the subordinate objects and I can just use POJOs there. I believe this approach would work with the Model-Driven idea, but it feels a little odd to me to mark up the domain object with HTTP-specific details.... then again the domain object is already annotated with Hibernate/JPA annotations so its not like its really 'pure'..
Comments, alternate approaches?
[ March 24, 2008: Message edited by: Eric Nielsen ] [ March 24, 2008: Message edited by: Eric Nielsen ]