views:

175

answers:

5

Back story:

So I've been stuck on an architecture problem for the past couple of nights on a refactor I've been toying with. Nothing important, but it's been bothering me. It's actually an exercise in DRY, and an attempt to take it to such an extreme as the DAL architecture is completely DRY. It's a completely philosophical/theoretical exercise.

The code is based in part on one of @JohnMacIntyre's refactorings which I recently convinced him to blog about at http://whileicompile.wordpress.com/2010/08/24/my-clean-code-experience-no-1/. I've modified the code slightly, as I tend to, in order to take the code one level further - usually, just to see what extra mileage I can get out of a concept... anyway, my reasons are largely irrelevant.

Part of my data access layer is based on the following architecture:

abstract public class AppCommandBase : IDisposable { }

This contains basic stuff, like creation of a command object and cleanup after the AppCommand is disposed of. All of my command base objects derive from this.

abstract public class ReadCommandBase<T, ResultT> : AppCommandBase

This contains basic stuff that affects all read-commands - specifically in this case, reading data from tables and views. No editing, no updating, no saving.

abstract public class ReadItemCommandBase<T, FilterT> : ReadCommandBase<T, T> { }

This contains some more basic generic stuff - like definition of methods that will be required to read a single item from a table in the database, where the table name, key field name and field list names are defined as required abstract properties (to be defined by the derived class.

public class MyTableReadItemCommand : ReadItemCommandBase<MyTableClass, Int?> { }

This contains specific properties that define my table name, the list of fields from the table or view, the name of the key field, a method to parse the data out of the IDataReader row into my business object and a method that initiates the whole process.

Now, I also have this structure for my ReadList...

abstract public ReadListCommandBase<T> : ReadCommandBase<T, IEnumerable<T>> { }
public class MyTableReadListCommand : ReadListCommandBase<MyTableClass> { }

The difference being that the List classes contain properties that pertain to list generation (i.e. PageStart, PageSize, Sort and returns an IEnumerable) vs. return of a single DataObject (which just requires a filter that identifies a unique record).

Problem:

I'm hating that I've got a bunch of properties in my MyTableReadListCommand class that are identical in my MyTableReadItemCommand class. I've thought about moving them to a helper class, but while that may centralize the member contents in one place, I'll still have identical members in each of the classes, that instead point to the helper class, which I still dislike.

My first thought was dual inheritance would solve this nicely, even though I agree that dual inheritance is usually a code smell - but it would solve this issue very elegantly. So, given that .NET doesn't support dual inheritance, where do I go from here?

Perhaps a different refactor would be more suitable... but I'm having trouble wrapping my head around how to sidestep this problem.

If anyone needs a full code base to see what I'm harping on about, I've got a prototype solution on my DropBox at http://dl.dropbox.com/u/3029830/Prototypes/Prototype%20-%20DAL%20Refactor.zip. The code in question is in the DataAccessLayer project.

P.S. This isn't part of an ongoing active project, it's more a refactor puzzle for my own amusement.

Thanks in advance folks, I appreciate it.

A: 

As is mentioned by Kirk, this is the delegation pattern. When I do this, I usually construct an interface that is implemented by the delegator and the delegated class. This reduces the perceived code smell, at least for me.

x0n
I already checked out those patterns, and I've gotta be honest, just because they're "patterns", doesn't satisfy me. Whether it's a perceived code smell or not, I just can't bring myself to like them. So I'm on the hunt for a better solution. Thanks though, I appreciate the info.
BenAlabaster
+4  A: 

Separate the result processing from the data retrieval. Your inheritance hierarchy is already more than deep enough at ReadCommandBase.

Define an interface IDatabaseResultParser. Implement ItemDatabaseResultParser and ListDatabaseResultParser, both with a constructor parameter of type ReadCommandBase ( and maybe convert that to an interface too ).

When you call IDatabaseResultParser.Value() it executes the command, parses the results and returns a result of type T.

Your commands focus on retrieving the data from the database and returning them as tuples of some description ( actual Tuples or and array of arrays etc etc ), your parser focuses on converting the tuples into objects of whatever type you need. See NHibernates IResultTransformer for an idea of how this can work (and it's probably a better name than IDatabaseResultParser too).

Favor composition over inheritance.

Having looked at the sample I'll go even further...

  1. Throw away AppCommandBase - it adds no value to your inheritance hierarchy as all it does is check that the connection is not null and open and creates a command.
  2. Separate query building from query execution and result parsing - now you can greatly simplify the query execution implementation as it is either a read operation that returns an enumeration of tuples or a write operation that returns the number of rows affected.
  3. Your query builder could all be wrapped up in one class to include paging / sorting / filtering, however it may be easier to build some form of limited structure around these so you can separate paging and sorting and filtering. If I was doing this I wouldn't bother building the queries, I would simply write the sql inside an object that allowed me to pass in some parameters ( effectively stored procedures in c# ).

So now you have IDatabaseQuery / IDatabaseCommand / IResultTransformer and almost no inheritance =)

Neal
"Favor composition over inheritance." <-- A lesson always taught, easily forgotten, until forgetting it bites us in the behind. Great answer.
Jon Limjap
Conceptually, I don't disagree. Speaking from a hypothetical point of view though, I guess the point of the exercise here is to see just how far the DRY principle can be taken. For instance, can it be taken to such an extreme that the architecture is *completely* DRY? Or is there some limiting factor of OOP (at least in .NET) that prevents DRY being taken to that extreme?
BenAlabaster
Too much water will kill you, just like too little will. =)
Neal
That's a weird analogy: too much DRYness equated to too much water. ;)
Giraffe
A: 

I think the simple answer is... Since .NET doesn't support Multiple Inheritence, there is always going to be some repetition when creating objects of a similar type. .NET simply does not give you the tools to re-use some classes in a way that would facilitate perfect DRY.

The not-so-simple answer is that you could use code generation tools, instrumentation, code dom, and other techniques to inject the objects you want into the classes you want. It still creates duplication in memory, but it would simplify the source code (at the cost of added complexity in your code injection framework).

This may seem unsatisfying like the other solutions, however if you think about it, that's really what languages that support MI are doing behind the scenes, hooking up delegation systems that you can't see in source code.

The question comes down to, how much effort are you willing to put into making your source code simple. Think about that, it's rather profound.

Mystere Man
Inheritance should be a last resort. Multiple inheritance should be a last resort if you really really really know what you are doing (and even then it probably should be avoided). MI effectively says that c is an a and a b which probably means that c has way too much responsibility.
Neal
@Neal - Given that MI isn't even an option in C#, isn't that a moot point?
BenAlabaster
@Neal - I'm not sure what your point is. This is a theoretical exercise, not a practical one. I agree that in practice, MI is problematic. But it is a useful tool to achieve DRY.
Mystere Man
The comment was in respect to MI and using inheritance based thinking to drive out a solution. I think that for some reason or other we have become so deeply entrenched in using inheritance to solve our problems that we ignore other, usually cleaner, approaches.
Neal
Ok, but what you seem to be missing is that this is *not* a "solution". This is an exercise in theoretics, attempting to achieve something that you would never do in a "solution". By taking things to extremes that you would never do in real life, you learn more about how to address real life problems. I hope you understand the phrase "for the sake of argument" because that's exactly what this is. For the sake of argument, assume he's not trying to build a real solution, but instead explore the theoretical boundaries of the technology.
Mystere Man
+1  A: 

I think the short answer is that, in a system where multiple inheritance has been outlawed "for your protection", strategy/delegation is the direct substitute. Yes, you still end up with some parallel structure, such as the property for the delegate object. But it is minimized as much as possible within the confines of the language.

But lets step back from the simple answer and take a wide view....

Another big alternative is to refactor the larger design structure such that you inherently avoid this situation where a given class consists of the composite of behaviors of multiple "sibling" or "cousin" classes above it in the inheritance tree. To put it more concisely, refactor to an inheritance chain rather than an inheritance tree. This is easier said than done. It usually requires abstracting very different pieces of functionality.

The challenge you'll have in taking this tack that I'm recommending is that you've already made a concession in your design: You're optimizing for different SQL in the "item" and "list" cases. Preserving this as is will get in your way no matter what, because you've given them equal billing, so they must by necessity be siblings. So I would say that your first step in trying to get out of this "local maximum" of design elegance would be to roll back that optimization and treat the single item as what it truly is: a special case of a list, with just one element. You can always try to re-introduce an optimization for single items again later. But wait till you've addressed the elegance issue that is vexing you at the moment.

But you have to acknowledge that any optimization for anything other than the elegance of your C# code is going to put a roadblock in the way of design elegance for the C# code. This trade-off, just like the "memory-space" conjugate of algorithm design, is fundamental to the very nature of programming.

Chris Ammerman
Thanks Chris, that's very insightful. I'd already thought about this approach, and you're right, it does require a more "set based" tack. I'd not pursued this approach yet, but I think that might explore this approach next to see how far I can take that.
BenAlabaster
A: 

I haven't looked deeply at your scenario, but I have some thoughs on the dual-hierarchy problem in C#. To share code in a dual-hierarchy, we need a different construct in the language: either a mixin, a trait (pdf) (C# research -pdf) or a role (as in perl 6). C# makes it very easy to share code with inheritance (which is not the right operator for code-reuse), and very laborious to share code via composition (you know, you have to write all that delegation code by hand).

There are ways to get a kind of mixin in C#, but it's not ideal.

The Oxygene (download) language (an Object Pascal for .NET) also has an interesting feature for interface delegation that can be used to create all that delegating code for you.

Jordão