.Net Head

For any of you who know who Uncle Bob is, you probably know he is a “fanatic” about the idea that methods should do one and only one thing.  If you have any doubts about this, check out his recent blog on the matter.  However I put “fanatic” in quotes because I am in complete agreement with him on this (and most other) points.

There are lots of reasons for keeping your functions this tight.  Combined with good method names, it makes for great self-documenting code.  It makes code reuse a lot easier because you never have code that does one thing tangled up with code that does something else.  That in turn makes it easier to keep your code DRY; if code is easily reused, there is rarely any reason to duplicate it.  All of these points, and their relationships, are often discussed.

One relationship I have not seen mention of, though it is implied in the above, is the relationship between Do-One-Thing (DOT) and the Open Closed Principle (OCP).  This relationship has had opportunity recently to re-impress itself upon my mind, so I thought I would take some time to discuss it.

For those not familiar with OCP, the Wikipedia entry gives a concise explanation.  For a more in depth, applied look, see Uncle Bob’s treatment of it.  For the real short summary, here is the definition from the latter.

Software entities (classes, modules, functions, etc.) should be open for extension but closed for modification.

Another way to say this which gives the sense in which I wish to discuss it here is this: Given an existing class, if I need to modify the behavior of the class, I should be able to do so solely by means of subclassing it.  I should not need to make any modifications to the class itself.

To clarify, this is important in scenarios where the original class is used by code beyond that which needs the modification.   If all code that uses a class needs the modification (such might be the case with a bug fix) then by all means the original class should be modified.  But if a modification is needed by only some users of the class, then OCP comes into play.  In this case we want to leave the original class unmodified in order to ensure that the other users are not affected by the change.  The code which needs the change, and only that code, uses the new subclass.

In contrasts to OCP, what often happens is this:  I have a new feature to implement that requires a slightly different behavior from the FooBar class.  FooBar is used by other features of the application that rely on the current behavior.  I could just make a copy of FooBar and modify it to fit my needs, but DRY says that is bad, so I correctly decide to subclass FooBar and just override the method that needs to behave differently.  So far so good.

So I pull up the source code for the FooBar class and find the code that I need to override.  It is in the Transmorgify() method.  But now things get messy.  Transmorgify() doesn’t adhere to the Do-One-Thing principle.  It does several things.  But I only need to override one of them.  Now I am in a conundrum.  I have two options.

1. I can override the current Transmorgify() method and copy its current implementation into my override, and modify the part that I need modified.  That violates DRY.
2. I can refactor Transmorgify() by extracting each behavior into its own function as Uncle Bob demonstrated above–then override the new more narrowly focused method. But that means modifying the existing code and possibly breaking something–a potentially costly error.

As can be seen, the failure to adhere to Do-One-Thing resulted in a failure to adhere to the Open Closed Principle.  To put it succinctly, a class cannot be closed to modification if any behavior that might, in the future, need to vary independently of another behavior is implemented in the same method as that behavior.

That is an ideal statement.  Realistically, it may be hard to identify every potential vector for change let alone isolate them.  And even if it can be done, the ROI might not be there.  Uncle Bob himself acknowledges this in his treatment of OCP.  (See page 6 on “Strategic Closure.”)  But this really goes back to the question of how far to take Do-One-Thing.  If you can, “do it till you drop.”  But if time constraints and ROI issues impose, then at least go as far as you can and error on the side of excess.  Why? Because we are human and we tend to error the other way because it is easier and faster.  So trying to error on the side of excess is more likely to get us into an appropriate middle ground.

No matter how hard we try to avoid them, ultimately, we are going to encounter scenarios like the above.  When it happens, the best thing is to have a comprehensive set of tests in place and refactor the original class as needed.

Preface

This blog builds on information I found in Brad Wilson‘s excellent 5 Part blog on ASP.Net MVC 2.0 Templates and ModelMetadata.

The Problem

I have been working on my first MVC 2.0 web application for a few weeks now and recently hit a story where I needed a view with parent/child data on it. The situation is very similar to the classic invoice and line items scenario. The view needs to contain data from the parent object (the invoice) and a grid of data from the child objects (the line items). Simplistically the view model looks something like this.

public class Invoice
{
    public int? InvoiceNumber { get; set; }
 
    public DateTime? InvoiceDate { get; set; }
 
    public IEnumerable LineItems { get; set; }
 
    [DisplayFormat(DataFormatString = "C")]
    public decimal GrandTotal { get; set; }
}
 
public class LineItem
{
    public int Quantity { get; set; }
 
    public string Description { get; set; }
 
    [DisplayFormat(DataFormatString = "C")]
    public decimal UnitCost { get; set; }
 
    [DisplayFormat(DataFormatString = "C")]
    public decimal TotalCost { get; set; }
}

Notice that I am using DataAnotations, as described in Brad’s blog, to indicate that the decimal values should be formated as currency.

This works fine for the GrandTotal value in the main body of the invoice. Adding the following to the view displays a nicely formatted grand total.

<%= Html.DisplayFor(i => i.GrandTotal)%>

However, when it came to the children, I wanted to use the MvcContrib Grid helper to render the data as follows:

<%
var formatForUnitCost = "C"; //TODO: get this from the ModelMetadata.
var formatForTotalCost = "C";//TODO: get this from the ModelMetadata.
 
//Convert metadata format to format string of the form "{0:format}"
formatForUnitCost = "{0:" + formatForUnitCost + "}";
formatForTotalCost = "{0:" + formatForTotalCost + "}";
 
Html.Grid(Model.LineItems).Columns( column =>
    {
        column.For(item => item.Quantity);
        column.For(item => item.Description);
        column.For(item => item.UnitCost).Format(formatForUnitCost);
        column.For(item => item.TotalCost).Format(formatForTotalCost);
    }).Render();
%>

Notice the to-do items. I expected this to be an easy thing and ultimately it was, but figuring it out was not as simple as I had hoped for. And that is the reason for my blogging today. How does one get the metadata for objects in collections?

Background and Initial Attempts

To get the ModelMetadata object for the model itself is easy. Just as you can call ViewData.Model to get the model (or simply this.Model to get a strongly typed reference if you are using a strongly typed view), you can call ViewData.ModelMetadata to get the metadata. (Sorry there is no this.ModelMetadata equivelent for strongly typed views.) So if I wanted the display format for the GrandTotal property, I could get it like this:

var formatForGrandTotal = ViewData.ModelMetadata.Properties
    .Where(p => p.PropertyName == "GrandTotal")
    .Single().DisplayFormatString;

The same does does not work so well if you replace “GrandTotal” with “LineItems,” which was my first attempt. This returned a Metadata object for the LineItems enumerator. Enumerators however don’t have any properties and thus the following returns zero.

return ViewData.ModelMetadata.Properties
    .Where(p => p.PropertyName == "LineItems")
    .Single().Properties.Count;

I tried changing the type of LineItems from IEnumerable<LineItem> to LineItem[]. This was a false improvement. The ModelMetadata object for the LineItems property now had some properties of its own, but they were just the standard properties of any array type: Length, LongLength, IsFixedSize, IsReadonly, IsSynchronized, etc. There was nothing that would provide access to the metadata for the contained types. No matter what I tried, I could not find a way to go through a collection property using the ModelMetadata interface and get metadata for the contained types.

I posted an explanation of the problem and a request for help on the ASP.net MVC forum and Brad himself was kind enough to reply but couldn’t provide me with an answer.

Attempt Two

I did some more digging and found a blog on extending MVC by implementing one’s own ModelMetadataProvider. This looked promising as a way to create and serve up my own ModelMetadata subclass. The subclass would have a property that, in cases where the current model type was a collection, would be populated with the ModelMetadata for the contained types.

As I went down this road, I became less enthusiastic. Creating a subclass that had a new property on it meant I would have to cast the ModelMetadata instance returned from ModelMetadata.Properties to my new type. That seemed messy and non-standard. MVCs own ModelMetadata subclassing did not extend the interface but only modified behavior.

Worse, I would need to subclass the the DataAnnotationsModelMetadataProvider as well. That should not be a big deal; but looking at the source code, it was clear that I would have to copy and paste most of the code out of its CreateMetadata method into my override because the implementation didn’t follow the do-only-one-thing rule. CreateMetadata did lots of things and I needed to override only one of them, namely the instantiation of the ModelMetadata instance so as to instantiate an instance of my ModelMetadata subtype. I don’t like duplicate code, but I especially don’t like it when I only own one half of it. It makes keeping things synchronized all the harder. (As an aside, this is why the do-only-one-thing rule is a prerequisite to adhering to the open-closed principle. The real problem here is that DataAnnotationsModelMetadataProvider isn’t open to extension (at least not cleanly) without requiring modification.)

The Solution

All of these concerns turned out to be irrelevent. In getting into the MVC source code, I realized I could very easily get the information I wanted without implementing anything.

As it turns out, the abstract base class ModelMatadataProvider has several methods on it that are used by the MVC framework to build out the metadata graph for the given model. One of these methods is:

public abstract ModelMetadata GetMetadataForType(
    Func<object> modelAccessor, Type modelType)

All I needed to do was get a reference to the ModelMetadataProvider currently being used by the framework and call that method with typeof(LineItem) and wha-la, I would have the metadata I was looking for. Getting the reference to the current provider is easy. It is available through the static property ModelMetadataProviders.Current.

While it is true that I didn’t have to implement anything, I did write the following extension methods to make things easier.

//Usage: someModel.GetMetadata()
//Gets the metadata for the type someModel.GetType().
public static ModelMetadata GetMetadata(this TModel model)
{
    return ModelMetadataProviders.Current.GetMetadataForType(
        () => model, typeof(TModel).);
}
 
//Usage: someModel.GetMetadataForProperty(m => m.SomeProperty)
//Gets the metadata for the type someModel.SomePropery.GetType().
public static ModelMetadata GetMetadataForProperty(
    this TModel model,
    Expression> propertySelector)
{
    var propertyName = propertySelector.GetReferencedPropertyName();
    return model.GetMetadata().Properties.Where(
        p => p.PropertyName == propertyName).Single();
}
 
//Usage: ViewModel.GetMetadataForType()
//Gets the metadata for the type MyModel.
public static ModelMetadata GetMetadataForType(
   this ViewDataDictionary model)
{
    return ModelMetadataProviders.Current.GetMetadataForType(
        null, typeof(TModel));
}
 
//Usage: ViewData.GetMetadataForProperty(m => m.SomeProperty)
//Gets the metadata for the return type of MyModel.SomeProperty.
public static ModelMetadata GetMetadataForProperty(
    this ViewDataDictionary model,
    Expression> propertySelector)
{
    var propertyName = propertySelector.GetReferencedPropertyName();
    return model.GetMetadataForType().Properties.Where(
        p => p.PropertyName == propertyName).Single();
}

Some notes on the code:

• The GetReferencedPropertyName is another extension method I wrote based on this great blog. For now just know that it takes a lambda expression of the form x => x.MyProperty and returns the string “MyProperty.”  You can find the code for this method here.
• I attached the last two methods to the ViewDataDictionary which makes them accessible as methods on the view’s ViewData property. This is just a convenience and they could be attached to most any type.

This finally allowed me to rewrite my code as follows:

var formatForUnitCost = ViewData.GetMetadataForProperty(
    item => item.UnitCost).DisplayFormatString;
 
var formatForTotalCost = ViewData.GetMetadataForProperty(
    item => item.TotalCost).DisplayFormatString;

Conclusion

In conclusion, I am not 100% satisfied with my solution. For one, any time I start working around a framework rather than through it, I begin to wonder if I am approaching something wrong. Maybe there is some reason why I should not structure my model this way, or there is a way to structure a the model that avoids this problem all together. On the other hand, this approach has the advantage of providing access to metadata in other scenarios where it is not readily accessible, such as when using the ViewData collection in lieu of an actual model.

Another concern is that passing null as the first argument to ModelMetadataProvider.GetMetadataForType may not work for all providers. It works with the DataAnnotationsMetadataProvider, at least in its current implementation. Thus I feel that the last two extension methods may be unsafe. One can sidestep this concern by using only the first two methods, though they have the minor disadvantage of requiring an instance of the model.

Ultimately, I would prefer to see a solution that is baked into the MVC implementation and not just bolted on from the outside. I like doing things the “normal” way. In this case that means accessing the metadata via the already provided ModelMetadata graph. Doing things the normal way is usually better down the road. Those who have to maintain the code will find it easier to understand if it sticks to standard approaches. Perhaps more importantly, if Microsoft makes changes to how MVC generates metadata, they probably won’t break the documented functionality, but they might break my code.

So in the end, if anyone out there has a better solution, I would love to hear from you. Until then, I have my metadata.

Well I have finally found the time to begin a blog.  Actually, I am not sure it was so much about finding the time as finding the topics.  For the last five years now I have been working on a big legacy project.  It wasn’t the sort of place where you do not do a lot of interesting work.  It was more the kind of job you read about in the anti-patterns section of software development books.  Nonetheless there was enough stuff going on to provide ample learning opportunities.  It was a very good place to think and study and hopefully grow wise enough to know when I have something worth saying and when I don’t.  We shall see.

In any event, I have been writing software, in one form or another since I was eight years old.  My first program computed Ohms Law on the TRS-80.  (I was never one to settle for Hello World.)  My first “commercial” application was a subscription management and billing application for my paper route.  It was written in Commodore Basic on the venerable C64 and served me well for several years.

Professionally, I have been doing software development since 1993, starting out in FoxPro 2.0, then Visual Basic, and starting in 2004, C#.  I have done everything from DOS based UI design to windows API programing, COM services to Web Applications, Perl CGI to .Net MVC.

Things I have not done are OS and driver development, no Cobol nor main frame programming.  I have never done any deep math stuff like simulations or modeling.  Part of this is because my degree is in Philosophy not Computer Science.  But Philosophy is great for critical thinking and communication skills so its not been too much of a handicap.

Living in D.C. suburbia, I have generally worked on very large long-running federal projects where I often gravitate towards tackling the messy problems others don’t want to and towards trying to bring discipline to cowboy programming practices.  Early on, I would often be used as a firefighter, sent in to fix the crisis of the day.

What I have gotten good at over the years is object oriented development, writing flexible, reasonably clean, self-documenting code and the like.  I am keen on agile for both technical and non-technical reasons but don’t have a lot of formal experience in it.  I discovered agile several years back, and in the process realized that a lot of the tactics and tricks that I was using to make software development work in my waterfall world, and thought of as my shortcuts for not being able to do things the right way, were actually espoused practices in Agile.  It was sort of an awakening to the idea that I really did know what I was doing after all. In a sense, I have been doing gorilla agile for a long time without know it.

Very recently, I have been given the opportunity to lead a new team doing agile development on a non-government greenfield .Net application.  So this is my big chance and as such and I am very excited to see how it goes and pass on anything that I learn.  It is also my first chance to use .Net 3.5, various open source tools and the like so expect to see some blogging about those topics too.