Single responsibility principle

At the current project I'm working on we've made a class with one of the most meaningless names I've ever encountered: Preprocessor. It's used to combine different parts of xml into a final xml document, but you probably can not tell that by the name of it. This class has two problems that I'll write about: it has a bad name, and it does too much so we couldn't find a good name for it.

public class Preprocessor
{
   public PreprocesResult Proces(PreprocesInput preprocesInput)
   {
   }
}


Preprocessor is a bad name for our class, because it doesn't mean anything in our domain. If we were building a system that had something to do with audio or video, then preprocessor maybe wouldn't be a terrible name. But we are building a system that manages documents and publications and the workflow involved. And the name preprocessor doesn't say anything about what is being processed, or how, or what the output will be. Is it preprocessing documents, or publications or something else?

XmlPreprocessor would be a better name. Still not good, but at least it would say what is being processed.

If multiple parts of xml would be put together, then XmlJoiner would be a better name, or XmlCombiner.

Why couldn't we choose a good name for our class? Because this class does too much. This is a list of things the class does when you call the Process() merhod:

• Add Metadata to the given XML document;

• Add header element to the given XML document;

• Get all referenced to images from content, get image data from database and disk;
• Calculate width/height ratio from image;
• Change image-values in the given XML document;

• Get organisation that created the given XML document;
• Get logo image of organisation, save to temp folder and save reference to database;
• Calculate width/height ratio from image;
• Add Logo-element to the given XML document;

• Get XSL file from disk;
• Execute XSL transformation on the given XML document;

• Save final version of the created XML document to temp folder and save reference to database.

As you can see the class does quite a few different things. Some are related to xml, some are related to getting data from disk or a database and some are related to calculating things like width/height ratio. And all are necessary to create the final XML document. But not all things should be put in one class. Let's look at every function of the class that is really related to modifying XML:

• Add Metadata to XML;

• Add header element to XML;

• Get organisation that created the document;
• Get logo image of organisation, save to temp folder and save reference to database;
• Calculate width/height ratio from image;
• Add Logo-element to XML;

• Get all referenced to images from content, get image data from database and disk;
• Calculate width/height ratio from image;
• Change image-values in XML;

• Get XSL file from disk;
• Execute XSL transformation;

• Save final version of XML to temp folder and save reference to database;

Now that we've identified the functions that are related to XML we can put them into another class:

public class SourceXDocumentCreator

{

   public XDocument AddMetadata(XDocument xdoc, string metadata)

   {

      // Add metadata to XDocument

      // return XDocument

   }

   public XDocument AddHeader(XDocument xdoc, string title)

   {

      // Add header with title  to XDocument

      // return XDocument

   }

   public XDocument AddLogo(XDocument xdoc, string name,

                            double width, double height)

   {

      // Add logo to XDocument

      // return XDocument

   }

   // etc.

}

The name is much better now, as it clearly states that the class can be used to create some Source XML Document. (There probably are alternatives that are even better.)

If we ever need to extend or modify the SourceXDocumentCreator class, it's much easier to decide how to do it. As it's evident that saving or loading files from disk or database shouldn't be in there, opposed to the first class Preprocessor, which did exactly that and much more.

Also calculating ratio's doesn't belong in SourceXDocumentCreator.

But adding an element to the underlying XDocument does belong in the SourceXDocumentCreator class. And so does the function of changing a specific value in the underlying XDocument. 

Conclusion

This article showed that gving a class a good name, and making sure that a class doesn't do too much (does only one thing), helps in making it easier to decide if and how to modify it.

For more information about the Single Responsibility principle see:
http://en.wikipedia.org/wiki/Single_responsibility_principle