Getting things done – Sometimes simpler is better

I’m working on a new project for my work.  I’m using S#arp Architecture started by Billy McCafferty, which is a framework using ASP.Net MVC, NHibernate for database persistence and Ninject for dependency injection.  It’s really shaping up to be a great framework and I’m learning a lot.

With NHibernate, you often come to the point where you have to map subclasses to your database.  The two prevailing strategies are one table per class or one table per class hierarchy.  You can look up any several dozen documents on these strategies.  For my project I chose one table per class hierarchy for one of my classes and its subclasses.

If you’re not familiar with Hibernate mapping documents, if you have one table per class hierarchy you have a single table storing the parent class and any subclasses all together.  This has its advantages, however, you have to have a column in the table (the discriminator) which holds a value telling Hibernate which class this record belongs to.  For instance, if I have a parent class of Person with two subclasses of Employee and Client, my discriminator field may hold “P” for Person, “E” for employee and “C” for client.

S#arp Architecture makes use of Fluent Nhibernate, which allows you to create Hibernate mapping documents in code in a fluent style.  Very nice.  It makes full use of generics and has been a pleasure to use once I dusted off my knowledge of mapping documents.

The problem is Fluent has a bug where it puts in discriminator elements for the subclasses.  There should only be one element describing the discriminator field and that goes in the main class definition.  For some reason Fluent also puts it in the subclass definitions causing Hibernate to throw an error.

So, what do you do?  Well, the great thing about open source is you can open up the code and find the problem.  I did just that but I hit another snag.  I love using Generics in my code, however, I’m not experienced at all at creating classes that use them.  After spending about a half an hour to find the problem and think about how to solve it I realized I didn’t have time to refactor the bug and relevant pieces to a logical usage.

So, I did the next best thing.  I added two lines of code to remove the element during the generation and fixed the issue.

The code to generate and return the mapping in xml ends with this:

1: writeTheParts(classElement, visitor);
2: return document;

So, I added the following:

1: writeTheParts(classElement, visitor);
2: foreach (XmlNode discriminator in document.SelectNodes("//subclass/discriminator"))
3:   discriminator.ParentNode.RemoveChild(discriminator);
4: return document;

In the code above, document is an XmlDocument type.  I used an XPath expression to find all the subclass elements that had a child discriminator element and then removed them.  The nice thing is this doesn’t affect any code that doesn’t have the bug.  It’s fairly clean and doesn’t get in the way at all.

While this certainly isn’t a true fix it does solve the problem in a very orderly and understandable way.  I still think it’s a hack but it helped me get back to my project.  I did submit the test (always create a test first!) and the patch to the project incase they wanted to fix the bug.  I also suggested that it still be refactored correctly by someone who is a little more experienced with Generics and the project.

So, in a case like this, I’d advocate simple when the solution is fairly clean, does exactly what it is supposed to do and there may not be time (or experience) for the alternative.