Archive for May, 2008

Choosing composition over inheritance: yet another example!

Julien on May 30th 2008

A friend of mine recently asked me if I though that having an inheritance hierarchy with a depth of 10 classes was acceptable or not. I don't think it is. If you take a class at the bottom, any change to the 9 classes above can introduce a breaking change. It can quickly become a maintenance nightmare!

Most of the time, if you have that kind of hierarchy in your application, it means that you chose inheritance over composition. It's probably also a code-smell indicating that your class is doing too much. To avoid that, let me show you how it is possible to extend a class without using inheritance.

Let's assume that we start with the following code. I know it's very basic, however it's still enough for our demonstration.

  1. abstract class TripBase
  2. {
  3. private readonly string _from;
  4. private readonly string _to;
  5.  
  6. public string From
  7. {
  8. get { return _from; }
  9. }
  10.  
  11. public string To
  12. {
  13. get { return _to; }
  14. }
  15.  
  16. protected TripBase(string from, string to)
  17. {
  18. _from = from;
  19. _to = to;
  20. }
  21.  
  22. public abstract DateTime CalculateEstimatedArrivalTime();
  23. }
  24.  
  25. class CarTrip : TripBase
  26. {
  27. public CarTrip(string from, string to)
  28. : base(from, to)
  29. {
  30. }
  31.  
  32. public override DateTime CalculateEstimatedArrivalTime()
  33. {
  34. return DateTime.Now.AddHours(15);
  35. }
  36. }
  37.  
  38. class PlaneTrip : TripBase
  39. {
  40. public PlaneTrip(string from, string to)
  41. : base(from, to)
  42. {
  43. }
  44.  
  45. public override DateTime CalculateEstimatedArrivalTime()
  46. {
  47. return DateTime.Now.AddHours(2);
  48. }
  49. }

We have a BaseTrip abstract class and we want to extend it by having 2 sub classes: CarTrip and PlaneTrip. In a real application, we would have some kind of algorithm to calculate or fetch the transportation time and it would probably be very different for each class. In this example, to simplify, we just return a hard coded value.
This code is not bad, however it's not perfect either. For instance, are we sure that calculating the transportation time between 2 city is the responsibility of the CarTrip and the PlaneTrip class? As a matter of fact, if we want to add a new MotocycleTrip class that would have different properties but the same algorithm than CarTrip to calculate the transportation time, we would need to extract a new abstract superclass that we would probably call RoadTrip... It doesn't look good...

Here is a refactored version of these class, using composition instead of inheritance:

  1. class Trip
  2. {
  3. private readonly string _from;
  4. private readonly string _to;
  5. private readonly ITransportationMode _transportationMode;
  6.  
  7. public string From
  8. {
  9. get { return _from; }
  10. }
  11.  
  12. public string To
  13. {
  14. get { return _to; }
  15. }
  16.  
  17. protected Trip(string from, string to, ITransportationMode transportationMode)
  18. {
  19. _from = from;
  20. _to = to;
  21. _transportationMode = transportationMode;
  22. }
  23.  
  24. public DateTime CalculateEstimatedArrivalTime()
  25. {
  26. return DateTime.Now.Add(_transportationMode.CalculateTransportationTimeBetween(_from, _to));
  27. }
  28. }
  29.  
  30. internal interface ITransportationMode
  31. {
  32. TimeSpan CalculateTransportationTimeBetween(string from, string to);
  33. }
  34.  
  35. class CarTransportationMode : ITransportationMode
  36. {
  37. public TimeSpan CalculateTransportationTimeBetween(string from, string to)
  38. {
  39. return new TimeSpan(15, 0, 0);
  40. }
  41. }
  42.  
  43. class PlaneTransportationMode : ITransportationMode
  44. {
  45. public TimeSpan CalculateTransportationTimeBetween(string from, string to)
  46. {
  47. return new TimeSpan(2, 0, 0);
  48. }
  49. }

In a few words, I've extracted the calculations in their own classes that implement an ITransportationMode interface. They are injected in the Trip class through the constructor (but we could also use a setter injection or a method injection).

This implementation improves the code in several way:

  • The calculation of the transportation time is done in a new class, it's not polluting our Trip class any more. It's a lot easier to add new algorithms, we just need to add a new implementation of ITransportationMode. It can evolves independently of the Trip class. It's called the separation of concern principle.
  • We can more easily configure our Trip class. Before, wa had to create a new instance of a TripBase sub class to change the way the calculation was done, now we can just change the instance of ITransportationMode while keeping the same Trip object.
  • We can have several subclasses of Trip using the same algorithm without introducing a new abstract class (such as RoadTrip). Therefore, composition helps us to keep our hierarchy of class flat.

kick it on DotNetKicks.com

Filed in General development | No responses yet

The null singleton

Julien on May 24th 2008

I'll say it straight: I really don't like the Singleton pattern. I think this pattern was great a few years ago, before we started to use things such as dependency injection or dependency lookup. Unfortunately for me, as of today, it's probably the most used pattern in software development. To be fair, this pattern can be useful in several scenarios, but please... not everywhere! (For the record, the a singleton is a pattern that ensures that you have only one instance for a specific class. This instance is created "on demand", the first time the class is used.)

The main problems with singletons are obvious:

  • You don't control when an instance is created, so if you have lot of them you have no idea in which order the instances will be created. Not an issue on small software, but more worrying when you have a few hundred thousands of lines.
  • 99% of the time, the implementation that is used binds us to a specific class. Therefore you have either to refactor the singleton or use a tool like TypeMock if you want to unit test your code. (Or you can have a empty class that holds the static instance and returns it casted as an interface... But doing that results in a multiplication of almost useless classes)
  • Dependencies are not obvious (which can also be a problem with dependency lookup). If you have type A that uses a singleton of type B, nothing in A signature will show you the dependency. It might be a problem if you're not aware that using A results in a hundred of round trips to the database through B!

However, I recently discovered a pattern that is even worse than the Singleton: I call it the "Null Singleton"! It's a very simple concept: the singleton can fail to instantiate itself if some dependency (a config file, a database, or whatever) is not available. In that case, the instance returned by the singleton will be null. Let's see an example:

  1. class NullSingleton
  2. {
  3. private static NullSingleton _instance;
  4.  
  5. public static NullSingleton Instance
  6. {
  7. get
  8. {
  9. if (_instance == null)
  10. {
  11. if (CanAccessMyDependency())
  12. {
  13. _instance = new NullSingleton();
  14. }
  15. }
  16.  
  17. return _instance;
  18. }
  19. }
  20.  
  21. private static bool CanAccessMyDependency()
  22. {
  23. // check if you can connect to the database for instance.
  24. }
  25.  
  26. private NullSingleton()
  27. {
  28. }
  29.  
  30. }

So what is wrong with that?

  • If the dependency is unavailable, the NullSingleton will check again the dependency each time something will access it. It will probably be very slow (usually, it will be an out of process call).
  • Each time you try to get an instance, you'll have to check for a null reference which makes the code noisy
  • And finally, it's breaking the usual expectation about singletons (that it should return 1 instance)

Do yourself a favor: don't use a "Null Singleton" :-)

kick it on DotNetKicks.com

Filed in General development | 2 responses so far

Spec#: should we really wait for it?

Julien on May 13th 2008

A friend of mine just posted about his desire to have Spec# ready for production. Funnily, many people in Alt.net expressed a similar desire in the past weeks (See that or that).

I don't have any problem with Spec#. On the contrary, I believe that it's a great addition to the language, one that would result in a significant improvement in code quality. However, I also don't think we should expect anything soon for various reasons:

  • Spec# is only a research project. Even if Microsoft did publish previews, I don't think they've communicated any plan to have a RTM yet.
  • Spec# is an addition to C#. However, there's now a lot more than C# in .NET, and I think that it would be problematic to add this feature to C# and not to vb.net for instance (which is, believe it or not, more used than C#). Actually, what happens if you build an assembly with Spec# and use it from C#/Vb.net? I don't have any clue but it would be interesting to know.
  • It will be difficult for Spec# to enter mainstream, enterprise will be reluctant to migrate to it. Moving from .NET 2.0 to .NET 3.5 is absolutely painless and still, do you know many companies that are actively working with .NET 3.5?
  • Spec# adds several new keywords which will be as many new concepts to understand before being able to work with it. With the recent additions to the framework, I'm starting to consider that there's already too much to learn for the average developer! Just an example: how many of your colleagues know what's the meaning of the "??" operator in C#? Or another one: I had to explain what was a Nullable only a few days ago :). If you add Linq, WCF, WPF, Entity Framework...

The bottom line is that I believe that we should focus our efforts on improving the way we do defensive programming with the framework as it is, and not hope for a white knight. Even if they are not perfect solutions, I think we can cover a lot of ground by systematically checking the inputs, checking the post conditions/side effects with unit testing, and checking that our object are satisfying invariants.

Filed in .NET | One response so far