Jon Flanders throws a little vitriol at the WCF team fore their design of the abstract ServiceHostBase class, calling the current design “stupid”. Normally, I’m not one to take up for the WCF team. I’ve spent many an hour banging my head against WCF for different reasons but exactly how Jon describes. However, in this case, the WCF team looks caught between a rock and a hard place, where the rock is correct behavior and the hard place is the way CLR object construction works.
The crux of Jon’s beef is that if you build your own ServiceHost by inheriting from SeviceHostBase, it’s not enough just to override the abstract CreateDescription method. You also have to call the “magic” InitializeDescription method in your derived class constructor. If you don’t, CreateDescription never gets called. CreateDescription returns a ServiceDescription instance which is to route messages from the channel layer to the and service layer. Jon writes that “Creating a valid ServiceDescription and returning the list of implemented contracts is essential for making your ServiceHostBase-derived class work.” I.e. it’s really important. Yet, unless you remember to call InitializeDescription, this “essential” ServiceDescription doesn’t get configured correctly. Yep, I see how that might sound stupid.
But if this design is stupid, what would be a better design? After thinking about this specific problem a while, I’m don’t think there is a better design out there.
The question is, when is the right time to set up the service description? Jon called ServiceDescription “essential” to the operation of ServiceHostBase. That implies it should be configured during the construction of a new service host instance. It wouldn’t do to have ServiceDescription unconfigured for some period of time between construction and use. What if the ServiceDescription is invalid or the call to CreateDescription throws an exception? Then you’d be in a position where you could create what looks like a valid service host, but it would throw an exception when you tried to use it. You can see why the WCF team would want to avoid that scenario.
So if you want the service host to have a valid ServiceDescription by the end of construction, what choices do you have? Given that the ServiceDescription depends on derived class data, the only choice is to use a magic method! Here’s an example to demonstrate what I mean:
public abstract class Base { public Base() { } private string _importantThing; protected abstract string CreateImportantThing(); protected void InitImportantThing() { _importantThing = CreateImportantThing(); if (_importantThing == null) throw new Exception(); } public string ImportantThing { get { return _importantThing; } } } public class Derived : Base { private object _data; public Derived(DateTime dt) { _data = dt; } protected override string CreateImportantThing() { return _data.ToString(); } }
I’ve tried to distill out the relevant parts of ServiceHostBase. In the example, Base stores some important thing that gets created by the derived class based on data that’s passed into the derived class’s constructor. Remember, we want the class to be fully configured by the end of the constructor. If CreateImportantThing throws an exception or returns null, we want to know about it right away when the object is created.
In the code above, the magic method InitImportantThing never gets called and thus the _importantThing field never gets setup. This roughly corresponds to Jon’s scenario where he didn’t know to call InitalizeDescription. And like WCF, we can make this sample work by inserting a call to InitImportantThing at the end of Derived’s constructor.
You might be tempted to put the call to InitImportantThing in Base’s constructor. But that won’t work because Base’s constructor runs before Derived’s constructor does. Thus, Derived’s _data field will still be null and the call to CreateImportantThing throws a null reference exception.
The final option would be to place the a call to InitImportantThing in ImportantThing’s property get method, if _importantThing is null. This defers construction of _importantThing until the first time it’s used. By this time, the derived class constructor will have run and so the derived class data will be available for use. This eliminates the magic method call, but it means we don’t know if the instance is valid until we go to use it – i.e. the scenario we were expressly trying to avoid.
So basically, the looks like the WCF team had two choices:
- Err on the side of caution and require calling InitializeDescription in the ServiceHostBase derived class’s constructor.
- Err on the side of easy extensibility and call InitializeDescription the first time the ServiceDescription.
Put that way, what the WCF team ended up doing doesn’t seem so stupid. This is especially true when you consider that the vast majority of people aren’t creating their own service hosts anyway. It would have been possible to do both: explicitly call InitializeDescription in ServiceHost’s constructor but also have an implicit call in ServiceDescription property get if the field was null. But I’m not on the product team, so I don’t know what other tradeoffs that implies. Checking for a null reference seems like no big deal, but I don’t know how often this property gets called.
One other point: even though I don’t think this design is stupid, I agree 100% with Jon that the exception is misleading.The way it’s written, the immediate assumption is that your implementation of CreateDescription is bad, not that you forgot to call InitializeDescription. It turns out that ServiceHostBase is already tracking wether InitializeDescription has been called via the initializeDescriptionHasFinished field. So why can’t it throw an exception like “Dude, you forgot to call InitializeDescription” when that field is false? It wouldn’t make the design any cleaner, but it would have saved Jon hours of digging thru the implementation of ServiceHost and ServiceHostBase in Reflector.