Archive for the 'General development' Category

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. public 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 | 13 responses so far

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

Keep your objects in a consistent state

Julien on Apr 12th 2008

Part of a good object oriented design is to keep the objects in a correct state.

For instance, in the financial world, we have financial instruments called "derivatives". According to Wikipedia:

Derivatives are financial instruments whose value changes in response to the changes in underlying variables. The main types of derivatives are futures, forwards, options, and swaps.

In the real world, a derivative can't exist if there isn't an underlying financial instrument associated. A future on the dow jones could not have been created without the dow jones in the first place. If we want to represent a derivative in a software, we must make sure that we follow the same rule.

So let's assume that I have the following class:

  1. class Derivative
  2. {
  3. private string _name;
  4. private Instrument _underlyingInstrument ;
  5.  
  6. public string Name
  7. {
  8. get { return _name; }
  9. }
  10.  
  11. public Instrument UnderlyingInstrument
  12. {
  13. get { return _underlyingInstrument; }
  14. }
  15.  
  16. public Derivative(string name, Instrument underlyingInstrument)
  17. {
  18. _name = name;
  19. _underlyingInstrument = underlyingInstrument;
  20. }
  21. }

Any new developer on a project that uses this Derivative class will mentally map the Derivative class to the corresponding financial concept. He will expect the UnderlyingInstrument property to return a non-null object. However, this is not guaranteed in the current implementation. As a matter of fact, this class can currently be used to only convey the name of a Derivative. If we wanted to do that, we would need to create a new class for that purpose only. So in the mean time, if we want our code to stay in a maintainable state, we need to make sure that each Derivative object will be constructed correctly. If not, different people will make different usage of the class.

In that case, ensuring the correctness of the object can be done very easily. We just need to do a bit of Design By Contract. So our constructor will become:

  1. public Derivative(string name, Instrument underlyingInstrument)
  2. {
  3. if(name == null || name.Length == 0)
  4. {
  5. throw new Exception("The name of the derivative can't be null or empty");
  6. }
  7. else if(underlyingInstrument == null)
  8. {
  9. throw new Exception("A derivative must have an underlying instrument");
  10. }
  11.  
  12. _name = name;
  13. _underlyingInstrument = underlyingInstrument ;
  14. }

Now you're sure that a fellow developer won't use your object to do a weird thing :).

Of course, you can improve the clarity of this piece of code significantly by using various techniques and frameworks(including Debug.Assert or your own Assert class). For instance, I would write something like that:

  1. public Derivative(string name, Instrument underlyingInstrument )
  2. {
  3. Guard.Against(name == null, "The name of the derivative can't be null");
  4. Guard.Against(name.Length == 0, "The name of the derivative can't be empty");
  5. Guard.Against(underlyingInstrument == null, "The underlying instrument of the derivative can't be null");
  6.  
  7. _name = name;
  8. _underlyingInstrument = underlyingInstrument;
  9. }

But that's a topic for another day!

Filed in .NET, General development | 2 responses so far