The DataTable wrapper design pattern considered harmful

A real life example of how a poorly thought out up-front design can hurt a project at a later stage.

I took over this part of the project as a project lead, along with my junior programming buddy, Andy.

After analyzing the preliminary requirements it became clear that the thesaurus management application wouldn't be anything revolutionary. Yes, the user interface called for some sophisticated display and editing tools. But the rest of the application was still a little bit boring - DataSets traveling back and forth between the rich UI client and our stateless thesaurus server.

I've been using interface-based .NET remoting with the server being hosted in IIS. This is, IMHO, the only practical remoting setup for database oriented, custom (i.e. no shrink-wrap) applications. (see links)

I didn't want the UI controls to directly manipulate the ADO.NET DataSets and DataTables returned from the server. (The database schema was likely to change and I wanted to avoid the "rippling" effect.) I also wanted to have the nice, client-side object model in a separate assembly, so I could delegate development of the various TreeViews, ListViews and other UI gadgets to Andy.

I could have employed typed DataSets, but I didn't want to. First, I don't like the generated code (it doesn't follow the recommended .NET naming conventions). Second, a typed DataSet is still a DataSet and I didn't want to clutter the object model with the plethora of DataSet methods, properties and events. Third, a typed DataSet doesn't really isolate the UI code from database changes. Fourth, I don't like code generators, because they make me feel I'm out of control. (I know, I know, it's rather simplistic. But hey, it's real. )

I've invented something I proudly called the "DataTable wrapper design pattern", where a server-generated DataTable is wrapped into a collection class, which delegates much of its functionality to the underlying DataTable.

As an example, let's have a ThesaurusClient class that exposes a GetToplevelDescriptors method. The method calls the server, which returns a DataTable with rows representing all Eurovoc descriptors that don't have superordinates:

Public Function GetToplevelDescriptors() As DescriptorInfoCollection
  Dim DescriptorsTable As DataTable = _ 
    ServerReader.GetToplevelDescriptors(...)
  Dim Collection As New DescriptorInfoCollection(DescriptorsTable)
  Return Collection
End Function

DescriptorInfo.vb

The key points are:

• DescriptorInfoCollection class holds a reference to the DataTable returned from server.
• DescriptorInfo class knows how to initialize itself from a particular row in the DataTable.
• The DescriptorInfoCollection class delegates enumeration to the underlying DataTable, generating DescriptorInfo classes "on the fly".

Efficient? Nope!

But I was still so vainglorious that I was effectively blind!

I went out to implement the Term class, TermCollection class, DescriptorType class, DescriptorTypeCollection class...all the classes followed the weird "DataTable wrapper" pattern. And, my poor buddy Andy followed with TermView class, DescriptorTypeView class...

Everything looked fine until we had to implement a thesaurus search feature, which should look like this:

• The user enters a search term into a Textbox and hits a Search button.
• The UI code invokes the ThesaurusClient.SearchTerm function, which goes to the server and wraps the returned DataTable into a TermCollection instance.
• The TermCollection instance is returned to the UI code, which displays it in a SearchResultsView control. The SearchResultsView control was a simple UserControl descendant that wrapped (I'm probably obsessed with the word ) a DataGrid control.

"Why didn't you bind to the TermCollection directly?" I asked.

"It doesn't work. The TermCollection doesn't implement the required IList interface." Andy replied.

Oops!

I sat down and tried to design the IList support with my "clever" wrapper pattern. It turned out to be a very useful exercise, because I finally realized that the design pattern is seriously flawed:

• It is inefficient. It creates a distinct set of objects for every "For Each" iteration through the "wrapper" collection.
• It is inflexible. The IList support couldn't be easily implemented by delegation (like the IEnumerable support), because I've used DataTables and not DataViews (DataTable doesn't support IList, only DataView does). Converting the design to using DataViews would be basically a rewrite.
• It doesn't isolate the database schema dependency at all! The Friend New(DataTable) constructors (where the dependency lies) are scattered throughout the various source files.

• Make the collection classes true collections that inherit CollectionBase and hold onto real objects.
• Copy the DataTables returned from the server into collections of client-side objects once and in one well-defined place (the DataTranslations friend class).

DescriptorInfo.vb

And the moral of the story? There are several of them, IMHO:

• Always finish your design before jumping to coding.
• Always measure your design against real requirements.
• Never allow your "proof of concept" prototype code to turn into the project itself.

Watch out!

© Palo Mraz, Monday, July 21, 2003

Links

http://msdn.microsoft.com/library/en-us/cpguide/html/cpconworkingwithtypeddataset.asp - MS documentation about typed DataSets.

http://dotnet.di.unipi.it/EcmaSpec/PartitionV/ - .NET framework naming guidelines (see partition D), and much more. A must read, IMHO. (You already have the spec on your hard drive - it is part of the VS.NET installation. On my machine it is in the D:\Program Files\Microsoft Visual Studio .NET\SDK\v1.1\Tool Developers Guide\docs folder.)

http://www.ingorammer.com/RemotingFAQ/ - Ingo Rammer's .NET remoting FAQ.

http://www.rds.com/doug/weblogs/webServicesStrategies/2002/11/18.html#a726 - a nice definition of loose coupling.

http://www.nrsr.sk/ - The official web page of the Slovak Parliament.