Reasons to subclass: Great, Good, Okay

Shout-out to brilliant developer (and my co-worker) David Potter, who--through hours of discussion--helped me arrive at this 'thought'.

FYI: I use a lot of WPF examples in here. Apologies to the non-WPF devs.

Back in June, I wrote a post titled: Don't subclass a Panel, unless you're making a Panel.

In that post, I outlined the 'great' reason to use subclassing (read, inheritance):

I read a great internal paper at MS once--written by one of those brilliant old-guard devs with decades of perspective. The point the author pushed: use inheritance only for polymorphism. Translated: when making MyGrid, only subclass Grid if you mean for people to treat your new MyGrid just like another Grid. Code (and a user, for the most part) that deals with Grid should be able to "deal" with MyGrid without any issues.

I use a strong word there: only.

Now with time and experience comes perspective.

I'm going to make this philosophy a bit more nuanced by introducing categories of subclassing: great, good, and okay.

My argument: there are times to use each, but the further you get away from 'great', the more trouble you can get in and the more careful and thoughtful you should be.

Great subclassing: for polymorphism.

Examples here: Stream (from mscorlib) and UIElement (from WPF).

Stream is worthless by itself. It's meant to be the base class for something else. But there are innumerable features of the CLR that deal with Stream generically, without much concern for any subclass.

UIElement is similar. Basically worthless by itself. Designed to be a baseclass. But things like WPF layout (Measure/Arrange) deal explicitly with UIElement, ignoring any details of any subclass.

If you create a Foo class and have other classes that take a Foo in a constructor or method argument, even though Foo is abstract or rarely used by itself you are playing in the 'great' end of the subclassing spectrum.

Good subclassing: cognitive simplicity

If 'great' subclassing is for software components, 'good' subclassing is for humans.

ContentControl (WPF) is a pretty good example. I can't think of any constructor or method that takes ContentControl as a parameter.

We could imagine Button and Label and ScrollViewer all re-exposing the common properties (Content, ContentTemplate, ContentTemplateSelector) without any changes in functionality.

But! It's really nice that users can look at Label and say "oh, that's a ContentControl" and immediately have a valid mental model of how to use it.

The baseclass, in this respect, provides a grouping paradigm for a user to understand classes of types.

While this is a nice feature, it has its issues.

ButtonBase is a good example.

ButtonBase is the 'clickable' widget in WPF.

Almost everything that is clickable subclasses ButtonBase.

ButtonBase subclasses ContentControl.

ContentControl is the 'content' widget in WPF.

Almost everything in WPF that has content is a content control. Well, almost. ContentPresenter has the same properties that ContentControl has regarding content (Content, ContentTemplate, ContentTemplateSelector).

But they don't share a baseclass. Why?

Well, because ContentControl is a Control.

Control is the 'template-able' widget in WPF. It lets you swap out the chrome.

ContentPresenter doesn't let you swap out the chrome, just the template for the data.

So, we could really imagine three separate classes, all of which derive from FrameworkElement.

• ClickElement: exposes the click properties/methods/events.
• ContentElement: exposes the content properties/methods/events (Yes, this class name already exists for text stuff. Play along...)
• TemplateElement: exposes the template properties/methods/events (this is what Control is now).

The fact that ButtonBase subclasses in a specific order is kinda arbitrary.

One could imagine wanting a template-able, click-able element that doesn't have content.

One could imagine wanting a click-able, element that holds content that isn't template-able.

Single inheritance forces framework authors to choose. This can be annoying if you want a re-mix of the available features.

A great example of this: TransitionPresenter (from the bag-o-tricks).

It has all of the content properties, but it's not a ContentPresenter or a ContentControl. (The way it does its visual tree is different than both of those controls.)

It turns out to be impossible to subclass ContentPresenter and it's confusing to subclass ContentControl (because TransitionPresenter doesn't use a ControlTemplate).

A really smart dev who was doing code clean-up actually changed the baseclass from FrameworkElement to ContentControl, thinking he was making things simpler.

This is bad! A user will then expect to be able to use the Template property to change the chrome. This is the same problem with making MyGrid from Grid unless you want people treating it like any other Grid.

This trap is usually because of the third type...

Okay subclassing: tactical code reuse

If you have 15 classes that all have a Foo, Bar, and Baz property, having a common baseclass just saves code. Less to document. Less to test. Less to mess up.

Fine.

But if sharing some common state is your only goal, just make sure you acknowledge (along with the rest of your team) that is why you're doing it. Put it in the code comments of the class.

In some respects this is less dangerous than the 'good' reason to subclass because the FooBarBaz class probably doesn't have much meaning or use beyond your tactical reasons. As soon as it does, though, you can fall into the MyGrid trap.

In closing

This was a really long write-up.

I guess my final advice is be careful.

As with anything in life, if you try to be clever for clever's sake, you'll get burned.

If you try to use every feature of a language or a framework just to add spice to your afternoon of coding, you'll get burned.

As I discussed in my post about patters and guidelines, always try to understand the implications of the tools/tricks you use when coding.

And that's all I have to say about that.

Happy hacking.

Did you find this interesting? Let me know. I'd hate to think I was riffing all this out just to make my tendinitis worse.

.NET 3.5 is here; IDataErrorInfo support in WPF seems to be popular

One of the things that sucks about being on the outside is the surprises.

I was blown off my chair when I heard .NET 3.5 released yesterday.

I talked about some of the new stuff in August.

My favorite feature (that I designed, at least) was IDataErrorInfo support in WPF data binding.

Rocky likes it.

Walk likes it.

Even God likes it. ;-)

You really should be thanking Dr. Bent, who made it.

And Bea, who made sure it worked.

Going beyond Monitor.Enter/Exit (C# lock) with LockHelper

Sample code: LockHelper.cs [6KB]

Debugging threading issues is painful.

Really painful.

One of the most painful problems: dealing with deadlocks--waiting forever on a Monitor.Enter (C# lock) and not knowing which thread already has the lock.

I tried to dig in on this on the CLR Base Class Library Forum. I was hopeful, since Java has a method [Thread.holdsLock(obj)] that returns the holder of the current lock

Sadly, .NET lacks this functionality.

Enter a can-do attitude: why not build it myself.

Enter LockHelper.

Goals:

• Identical semantics to Monitor.Enter/Exit
• Enable other Monitor features (Pulse, Wait)
• Close to identical syntax to C# lock
• Ability to know, given an instance of LockHelper, the current owning thread
• Other conveniences (CheckAccess and VerifyAccess)

Wrapping Monitor.Enter/Exit isn't that hard. As you see with the implementation, it's pretty easy to leverage IDisposable + the C# using statement to get syntax that is close to lock.

Instead of:

public void Foo()
{
    lock(m_lockObject)
    {
        //do stuff
    }
}

private readonly object m_lockObject = new object();

Do this:

public void Foo()
{
    using(m_lockObject.GetLock())
    {
        //do stuff
    }
}

private readonly LockHelper m_lockObject = new LockHelper("foo");

GetLock() returns a private class that implements IDisposable. The using block has identical try/finally semantics to the lock keyword. (See this article in the C# Programming Guide for details.)

I had to be religious about storing away Thread.Name for the current thread at all times. While debugging, I found that VS was often unable to evaluate the property of the owning thread.

So now I have enough plumbing to figure out which thread has a lock on my lock object. Cool.

What else?

Exposing Pulse and Wait was pretty easy. Beyond these basics, I stole the CheckAccess/VerifyAccess ideas from WPF.

Often times one will have private helpers in a class that access/modify the state of a class. Often times these methods (should) run under the assumption that they were called in the context of a lock.

The simple way around this is to re-lock the object inside the helper method, but this can be a source of deadlocks and really doesn't help maintain the implementation contract.

What one wants: a way to assert that the calling thread owns a lock.

Done!

CheckAccess returns true if the current thread has the lock. Otherwise, false.

VerifyAccess goes a step further and throws an exception if the calling thread doesn't own the lock.

Pretty cool, huh?

Note: I was not on the CLR team at Microsoft. I was not a developer at Microsoft, just a lowly PM. I'm pretty confident this works. At least I haven't hit any issues with it. Please be careful throwing this in your own projects. This is certainly out of the realm of advertised best-practices from MS.

It works for me, though.

Let me know what you think.

Happy hacking!

The event pattern in C# and funny coincidence

On Thursday, I wrote about the importance of software patterns, specifically the event pattern in C#.

That very afternoon I was going through some code written by a co-worker that leveraged this pattern.

To keep things simple, let's imagine it was like this.

public event EventHandler NewMessage;

protected virtual void OnNewMessage(MessageEventArgs args)
{
  EventHandler handler = NewMessage;
  if(handler != null)
  {
    handler(this,args);
  }
}

protected virtual void OnNewMessage(string message)
{
  EventHandler handler = NewMessage;
  if(handler != null)
  {
    handler(this,new MessageEventArgs(message));
  }
}

Pretty straight forward, right? See the problem?

This gets to the core of why understanding a pattern deeply is important. The pattern of having a protected virtual OnFoo method for your Foo event is to ensure that a subclass can peek at the event before it's fired. The implementation above will require the subclass to not only override both methods, but to understand that she must override both methods to make sure all raised events are captured.

This is making the job of the subclass writer much harder and likely more error prone.

I'm not saying one should get rid of the helper method. It's fine. It just needs to be tweaked.

protected void OnNewMessage(string message)
{
  OnNewMessage(new MessageEventArgs(message));
}

First, get rid of the virtual flag. This should be callable from a subclass, but there should only be one method to override.

Second, call the 'real' virtual method from the helper. This ensures that anyone who overrides OnNewMessage will get the desired result.

A clever developer might not like this, because she wants to avoid allocating the EventArgs unless there actually are handlers to call. Clever optimization, but it breaks the pattern.

Make sense?

Happy hacking.

Former MS co-workers done well: Amar & Robby [Videos]

Amar was my GPM (manager's manager) in the Longhorn days. (Yes, back when Vista had a lot more managed code and was going to change the world.)

He's one of the stars in the intro video for Google's openSocial. (Look for the Indian guy in the Brazilian soccer jersey.)

I can't wait to start hacking on this stuff.

The last time you may have seen him on video was when he was GPM of the RSS team at Microsoft. (Interviewed by Scoble, none the less. A lot can change in three years, huh?)

Robby is on channel9 again, chatting the goodness of the 'integrator' role in WPF/Silverlight development. He's a main guy at identity mine these days.

Robby and I worked together on the WPF team in the v1 days. The last time you may have seen him on channel9 was with yours truly. Although I'd rather forget the topic. :-)

The importance of patterns and guidelines in software development

I've been pulling my hair out on IDisposable, Dispose() and protected virtual Dispose(bool disposing) the last 24 hours.

I think I've figured it out. Finally!

I started writing a whole entry on it, but then I realized there is a whole prequel that needs to be written first. So...

We're talking about patterns and guidelines for software here, my friends. They exist to ensure consistent behavior and therefor consistent expectations. This is for direct users of your classes and also those who subclass your classes. The problem with patterns, though, is that they become customs. We do them without asking why we do them. Then we start questioning why we do them if we don't fully understand. We might even start tweaking them if they get cumbersome, which can cause unexpected consequences later on.

So it's always good to understand why (at least at some level) we implement a pattern.

My favorite example: events.

So you add a public event to your class:

public event EventHandler Updated;

And because you were told to, you implement:

protected virtual void OnUpdated(EventArgs args)
{
 EventHandler handler = Updated;
 if(handler != null)
 {
   handler(this, args);
 }
}

Like a robot. Do yo know why? Really?

Q: Why protected?
A: So a subclass can raise your event. The C# compiler is doing work behind the scenes. You don't see that the event really has both public and private surface area. Only the class that defines the event can raise it directly.

Q: Why virtual?
A: Because a subclass might want to intercept the event and take some action on it. It's cumbersome for a subclass to register an event listener on itself.

Q: Why create the 'handler' field in the method? Seems like a waste of a variable.
A: Threading. To ensure that between the check for null and the call, the event hasn't been unregistered. (But I don't have multithreaded stuff in my library, you say. Right. But might you tomorrow? Might a subclass? Might a direct user of your library?)

Q: Why not simplify the method signature to take no parameters? EventArgs is empty, right?. I should just handle that in the method, right?
A: No. Some subclasses might want to pass a different EventArgs to the method. They might want to pass more information via FooEventArgs. If you lock down the method signature, you are locking them out of extensibility. (Working on WPF, we actually got a bug about this with OnCollectionChanged in ObservableCollection).

See! Such a simple pattern, yet so many nuances!

You might do this work even if you haven't imagined any subclasses. You are doing it just in case.

In fact, the only good reason not to do this work is if you seal your class. Even then, what if the class gets unsealed in the future? Will you remember re-implement the pattern the right way?

Better to do it correctly from the start.

I need to get some work done now. In the next day or two, I'll post my discoveries on IDisposable.

In the mean time, ask yourself: am I cutting corners with patterns? Am I implementing patters which I don't fully understand?

Hmm...

Happy hacking.