Wednesday, April 1, 2009

Class Design Guidelines

Introduction

One of the most common tasks is to add a new class to a project. But what cares should be taken when writing its source code? I the last years I have moved from a product development mindset to a framework development mindset. Several new challenges arise from this shift, such as:

Thinking in the perspective of who is using the framework. A framework usage has at least two viewpoints:

  • The users of the products when there are user interfaces.
  • The programmers that build applications with it.

UsersAndCoders

In my opinion the best designs win because they:

  • Know the programming habits of the target audience
  • Name the class and methods so that they are the first thought.

If we think on what is in front of our eyes when coding in visual studio it is easy to see why. Most programmers don’t search the documentation when they want to do something. They type in the name of the class that sounds the best (first hint) and then the method that sounds the best. If a framework uses well chosen names the first hint will be the correct one most of the times, this makes it more productive, less frustrating and these things sell.

Intellisense

Abstractions

It is sometimes a good idea to associate a class to an abstraction, either another class but abstract or an interface. Typically inheritance is used when the association between the parent and the child class can be read as “is a” and interfaces are used when the association is read as “can do”, “is capable of”, “is able to” and so one.

In this article I will use a fictitious blog engine to provide for small examples an highlights about the ideas presented here.

   1: /// <summary>
   2:   /// Base class for all entries in the blog that provides a common API and 
   3:   /// attributes to all kinds of posts.
   4:   /// </summary>
   5:   /// <remarks>
   6:   /// This class will be specialized according the the formats of the Posts. Some of
   7:   /// the supported formats are RTF, HTML and PlainText.
   8:   /// </remarks>
   9:   public abstract class Post : IFullTextSearchable
  10:   {
  11:       /// <summary>
  12:       /// Validates the posted content, making sure it complies with all the blog
  13:       /// host rules and if not throws a <see cref="ValidationException"/>.
  14:       /// </summary>
  15:       public abstract void Validate();
  16:  
  17:       /// <summary>
  18:       /// Loads the post entry from the given value that should contain a serialized post.
  19:       /// </summary>
  20:       /// <param name="value">The string containing the serialized post object.</param>
  21:       public abstract void Load(string value);
  22:  
  23:       /// <summary>
  24:       /// Loads the post entry from the given value that should point to a text stream with a serialized post object.
  25:       /// </summary>
  26:       /// <param name="value">The  text reader pointing to a serialized post object.</param>
  27:       public abstract void Load(TextReader value);
  28:  
  29:       /// <summary>
  30:       /// Saves the post entry to a serialized representation, converting it afterwards to a string.
  31:       /// </summary>
  32:       /// <returns>The string containing the serialization result.</returns>
  33:       public abstract string Save();
  34:  
  35:       /// <summary>
  36:       /// Saves the post entry to a serialized representation, apending it afterwards to the given string writer.
  37:       /// </summary>
  38:       /// <param name="writer">The text writer that will be used to write the serialization result.</param>
  39:       public abstract void Save(TextWriter writer);
  40:  
  41:       /// <summary>
  42:       /// Tries to find matches for the given pattern.
  43:       /// </summary>
  44:       /// <param name="pattern">The pattern to search for.</param>
  45:       /// <returns>The hits that match the search pattern.</returns>
  46:       public IList<FullTextSearchHit> Search(string pattern)
  47:       {
  48:           if (pattern == null)
  49:           {
  50:               throw new ArgumentNullException("pattern");
  51:           }
  52:  
  53:           try
  54:           {
  55:               // wrap the given pattern in the corresponding object and validate it
  56:  
  57:               FullTextSearchPattern searchPattern = new FullTextSearchPattern(pattern);
  58:               searchPattern.Validate();
  59:  
  60:               // ask the inheritors to do the search
  61:  
  62:               var result = this.OnSearch(searchPattern);
  63:  
  64:               // don't trust the implementations to provide a valid value
  65:  
  66:               if (result == null)
  67:               {
  68:                   // return an empty search and assume the search had no hits
  69:  
  70:                   return new List<FullTextSearchHit>();
  71:               }
  72:  
  73:               // and if it is valid return it
  74:  
  75:               return result;
  76:           }
  77:           catch (InvalidPatternException ex)
  78:           {
  79:               throw new SearchException(Properties.Resources.SearchFailed, ex);
  80:           }
  81:       }
  82:  
  83:       /// <summary>
  84:       /// Called when a full text search is required.
  85:       /// </summary>
  86:       /// <param name="pattern">The validated pattern to search for.</param>
  87:       /// <returns>The hits that match the search pattern.</returns>
  88:       protected abstract IQueryable<FullTextSearchHit> OnSearch(FullTextSearchPattern pattern);
  89:   }

In this design there are two key points:

  1. Post is made abstract and will be extended through inheritance because all objects in this context share a common identity. They are posts, though they vary in format they always are posts.
  2. For the full text search I choose interfaces because I might want to do a full text search in the registered users. Maybe to find an email. So the objects that implement this interface share a capability, doing full text search.

Notice that in the documentation all the overloads have a similar starting sentence an then are specialized according to their input arguments.

Also notice how the implementation of search is handover to the classes inheriting post. I provide a abstract protected method where developers must implement the feature but do not trust the implementation and check for both input and output constraints in the public part. Virtual and abstract methods should be considered public and should not be trusted. When possible overloading them should not compromise the main features. For instance it is very common for developers to forget to call the base implementation.

Finally when accepting or returning class instances (specially collections) it is a good idea to use the most abstract representation possible, eg. a Stream instead of a FileStream. For collections I tend to use the ReadOnlyCollection<T> or interfaces like IEnumerable<T>, IQueryable<T> or IList<T> depending on the requirements.

There is one development style that I find annoying that consists in splitting a concept in an interface and its realization and do it for everything including classes that only carry data. This is a nightmare when you have to add a new feature into the product. We have to add methods, properties and events in multiple places.

I have this personal common sense rules:

  1. Objects that mainly carry data should not be hidden behind an interface. If I want to stop people from creating instances of a base class in a hierarchy I make it abstract.
  2. Objects that represent an algorithm or strategy to do something should be represented by an interface. I also find a common good practice to provide a base class for implementing the interface.

When I use dependency containers I tend to represent the components that are to be revolved by the container by interfaces.

Message Exchange Between Class Methods

In most cases methods have access to the state that is internal to the class. So one recurring issue is when should those state variables be passed as arguments or when should they be access directly. I haven’t yet found a good rule for this problem but learned the hard way this is a good place to spend some thinking. Any comments on this are most welcome.

Exceptions

When I don’t think on exceptions FxCop remembers me :).

There is a small group of exceptions provided by the .NET framework that everyone should now about and use:

- ArgumentNullException

- InvalidOperationException

This is a good practice because developers tend to learn when they apply and put the necessary handling logic without having to think to much on it. Habit always wins.

But besides these “universal” exceptions classes should throw exceptions that have something to do with they purpose and that tell either the user or the developer what they are doing wrong.

   1:     /// <summary>
   2:     /// The exception that is thrown when an invalid search pattern is entered.
   3:     /// </summary>
   4:     [Serializable]
   5:     public class InvalidPatternException : ApplicationException
   6:     {
   7:         /// <summary>
   8:         /// Initializes a new instance of the <see cref="InvalidPatternException"/> class.
   9:         /// </summary>
  10:         public InvalidPatternException()
  11:             : base()
  12:         {
  13:         }
  14:  
  15:         /// <summary>
  16:         /// Initializes a new instance of the <see cref="InvalidPatternException"/> class.
  17:         /// </summary>
  18:         /// <param name="message">The message.</param>
  19:         public InvalidPatternException(string message)
  20:             : base(message)
  21:         {
  22:         }
  23:  
  24:         /// <summary>
  25:         /// Initializes a new instance of the <see cref="InvalidPatternException"/> class.
  26:         /// </summary>
  27:         /// <param name="message">The message.</param>
  28:         /// <param name="innerException">The inner exception.</param>
  29:         public InvalidPatternException(string message, Exception innerException)
  30:             : base(message, innerException)
  31:         {
  32:         }
  33:  
  34:         /// <summary>
  35:         /// Initializes a new instance of the <see cref="InvalidPatternException"/> class.
  36:         /// </summary>
  37:         /// <param name="info">The object that holds the serialized object data.</param>
  38:         /// <param name="context">The contextual information about the source or destination.</param>
  39:         protected InvalidPatternException(SerializationInfo info, StreamingContext context)
  40:             : base(info, context)
  41:         {
  42:         }
  43:     }

The constructors in the above example are the minimal set to comply with the .NET framework conventions. Also it is important to mark exceptions as serializable making them transportable over the wire and over AppDomains.

When targeting the end-user one good practice is to start by building an exception hierarchy that divides the exceptions you are throwing in groups. There are at least two groups that all applications should have: the ones that can be presented to the user as is or the ones that should be shielded and presented with a generic message.

When this is done one it becomes easier to define a global exception handling policy (what gets silenced, what gets wrapped, what gets thrown). One can design a exception policy per group instead of per exception.

Also in N-Tier applications I find it useful to wrap exceptions in each layer. Each layer should add context about where and how the problem happened. With this in place UI developers can than extract the context information from the exceptions and build a nice dialog that clearly explains what was the error and what was being done. The maintenance team will also love you because they will get a lot more information.

Logging

What we are developing will one day leave our machine. Face it. When it does say good bye to the debugger. It is very rare to be able to remotely debug in a customer machine :). So your ability to isolate and fix a defect depends on how much information you can get about what was happing when the failure happened. So most products have some way of outputting their state changes and execution paths.This is one aspect of logging.

The other aspect of logging is when the users need to now what is happening in the product or what happened. This is the other aspect and is closely related with concepts like auditing.

Dependencies on other classes

This is far the most complex and the major fire starter in software, dependencies. Imagine you have an MVC based UI and an Application main class. Should the concrete view A access the application directly. I say hell no. In the pattern the view talks to the controller. Also I would try to have the controller as independent as possible from the Application. Maybe if it needs something about the application it can receive that in the moment of construction and be notified if eventually it changes.

There are some posts on this blog about this problematic:

Law of demeter.

Dependency injection.

Data Exposure

Classes that expose all their fields are either DTOs or a problem. Automatic properties are new nice feature in .NET but are a devil’s temptation for our laziness, if one fails to resist the public or protected modifiers. We build classes to achieve encapsulation. If it wasn’t for that maybe we could just put all program variables in a file and live with C.

This seams quite obvious but there are a lot of people ignoring this simple rule.

Once you expose something you increase the amount of testing required, you increase the chances of having to break something in a refactoring operation or off ending up with a design that is very hard to augment.

Events

The observer pattern is so important that .NET supports it natively, it is built on the programming language.

   1: /// <summary>
   2:     /// Contais data for the PostPublished event.
   3:     /// </summary>
   4:     [Serializable]
   5:     public class PostPublishedEventArgs : EventArgs
   6:     {
   7:         /// <summary>
   8:         /// Initializes a new instance of the <see cref="PostPublishedEventArgs"/> class.
   9:         /// </summary>
  10:         public PostPublishedEventArgs()
  11:         {
  12:         }
  13:  
  14:         /// <summary>
  15:         /// Gets or sets the user that did the post.
  16:         /// </summary>
  17:         public string PostedBy
  18:         {
  19:             get;
  20:             set;
  21:         }
  22:  
  23:         /// <summary>
  24:         /// Gets or sets the moment when the publication was made-
  25:         /// </summary>
  26:         public DateTime PostedAt
  27:         {
  28:             get;
  29:             set;
  30:         }
  31:  
  32:         /// <summary>
  33:         /// Gets or sets the title of publication.
  34:         /// </summary>
  35:         public string Title
  36:         {
  37:             get;
  38:             set;
  39:         }
  40:  
  41:         /// <summary>
  42:         /// Gets or sets the tags that where associated with the post.
  43:         /// </summary>
  44:         public string[] Tags
  45:         {
  46:             get;
  47:             set;
  48:         }
  49:     }

Each component should publish key notifications about changes to its state. There is no rule of thumb on what notifications should be included. One most consider what kind of information would be interested in. For this concept these where chosen:

   1: /// <summary>
   2:     /// Contains the logic for handling the posts in the blog.
   3:     /// </summary>
   4:     public abstract class BlogEngine
   5:     {
   6:         #region Events
   7:  
   8:         /// <summary>
   9:         /// Occurs when the post is published.
  10:         /// </summary>
  11:         public event EventHandler<PostPublishedEventArgs> PostPublished;
  12:  
  13:         /// <summary>
  14:         /// Occurs when a post is selected via a resolve url operation.
  15:         /// </summary>
  16:         public event EventHandler<PostSelectedEventArgs> PostSelected;

 

In most cases event are raised in a On<EventName> protected virtual method that takes the event class instance as argument. This allows for developers to include some logic before the event is raised.

   1: /// <summary>
   2:         /// Raises the <see cref="E:PostPublished"/> event.
   3:         /// </summary>
   4:         /// <param name="ev">
   5:         /// The <see cref="Pedrosal.DesignNotes.ObjectModels.BlogEngine.PostPublishedEventArgs"/> 
   6:         /// instance containing the event data.</param>
   7:         protected virtual void OnPostPublished(PostPublishedEventArgs ev)
   8:         {
   9:             if (this.PostPublished != null)
  10:             {
  11:                 this.PostPublished(this, ev);
  12:             }
  13:         }
  14:  
  15:         /// <summary>
  16:         /// Raises the <see cref="E:PostSelected"/> event.
  17:         /// </summary>
  18:         /// <param name="ev">
  19:         /// The <see cref="Pedrosal.DesignNotes.ObjectModels.BlogEngine.PostSelectedEventArgs"/> 
  20:         /// instance containing the event data.</param>
  21:         protected virtual void OnPostSelected(PostSelectedEventArgs ev)
  22:         {
  23:             if (this.PostSelected != null)
  24:             {
  25:                 this.PostSelected(this, ev);
  26:             }
  27:         }

Extensibility Vectors

Everyone gets to code a class that has to be extended somehow. Events are a nice extensibility feature but sometimes we really need to change the implementation of some features.

Specialization or inheritance is the process to change the behavior of an object (class) by making it less abstract than its base. For instance a FileStream is less abstract than Stream as we already now the stream is stored as a file.

When designing we should plan for these cases by:

Allowing the developers to override the features that lead to specialization by making it virtual.

Exposing some of the state of the object with protected properties.

Minimizing the effects that overrides can have on a object.

Avoid calling virtual methods on constructors.

As a programming rule of thumb the protected keyword for non sealed classes should be considered public in terms of testing and thrust.

Documentation

The summary should be a short simple phrase stating what is the purpose of the member. Documentation is not literature, it is a working tool as important as the IDE or the debugger. The phrase should go right to the point.

On overloads I find it better to start by documenting the longest overload and paste the documentation removing the unneeded parameters. Also in a overload it is common to use the same initial phrase to state what the method does and them add a brief description of how that method differs from the others. 

When overloading is a way to overcome the lack of default parameter values we can use the optionally word to describe what else you can do if you pass in the extra arguments.

Another good practice is to document the exceptions that are known to be thrown.

A really great tool to have installed in visual studio is GhostDoc.

Naming

Find the best name is the difference between something that developers will hate or be passionate with. The rule for naming is habit always wins. There are some patterns that are used everywhere. If the name you choose is the first thing that a developer starts typing when we needs that feature than it is a success.

Looking at the .NET framework is a good place to start. This doesn’t mean that it is the best in the world it just means that everyone is using it, they will eventually get it on their finger tips and when it is time to use your component those habits will guide them to your well chosen.

One example, if I design a set in C# and name the API as in STL C++ developers will love me but C# developers will hate me. By the way this is the second law of naming. Now your developers and their culture. Lean what is on their finger tips. Remember that in other things in life we like standards. Think on light switches, on cars and on roads. Now imagine if the guys designing these had our creative culture and kept changing the rules! Every day you would have to learn all this basic stuff again and at least for me it would be quite frustrating.

There are times where we challenge these orthodoxies and are really innovative but on these times we are doing it intentionally.

Implement IDisposable

Implementing this pattern in the main classes doesn’t hurt and I can come in hand. When applications get larger programmers often come to need a reliable way of releasing large hierarchies of objects. Disposable is the perfect way of achieving it. Also getting used to wrap IDisposable objects inside a using statement (when supported) is a good practice. I find it useful to implement even when I don’t use unmanaged resources.

No comments: