Archives

Pragmatic SOLID – Part 4 – The Interface Segregation Principle

Clients should not be forced to depend on methods they do not use.

TL;DR

  • Do not let client code use large classes which expose them to risk.
  • Split these classes into smaller classes (Single Responsibility Principle), or Interfaces.
  • Keep class interaction lean and focused.
  • Let interface design be dictated by the consuming clients, or by a logical service a subset of methods provides (e.g. Logging, UIConfirmation).
  • Interface Segregation does not necessarily mean using the Interface keyword, but it is designed for the purpose!
  • Automatically creating an Interface every time you create a class or method will lead to trouble.
  • An interface can also be too big – and may require segregating further.

What’s the business value?

By avoiding a monolithic object (or interface) from which developers pick and choose arbitrary methods, code is easier to decouple, test and maintain. There will generally be fewer (unnecessary) dependencies and less code to review when making a change to a smaller interface.  However, over-application of the principle can create a large amount of code complexity through over-simplification for no real gain; e.g. creating a low level  interface with just one method which is only consumed by one class.

Come again?

This principle ensures cleaner code and  prevents creation of one class to rule them all, which is typically knitted into the entire code-base. This scenario often occurs when a primary central object exists, and over time, developers add single methods to it in a hurry. They typically add their function, show some working software and walk away. Left unchecked, they or other developers follow suit. Soon there’s 10 new methods on a class, polluting its very fabric.

Any seasoned developer knows that code is read and maintained far many more times than written, so it is a fundamental principle to make sure we optimize for the reader and produce a good architecture that doesn’t leave a new starter scratching their head. Having to scroll through endless large function lists makes it harder to maintain. It is also no longer clear what the primary purpose of the class is, and developers will be less keen to refactor the class as it becomes larger and larger. If all client classes take this monolithic beast as their constructor argument, it is never clear what aspect they really depend on.

By applying interface segregation and only providing client classes with  the interfaces they need, developers can

  • introduce less risk when changing the code
  • easily plug in different implementations and have lean abstractions
  • easily provide fake implementations for testing

Example

You can use classes to provide interface segregation through the façade pattern – much like applying the single responsibility principle. However, the more typical way is to create an Interface, and declare that your class inherits all interfaces your class provides. In C# multiple inheritance of classes is not supported, but multiple inheritance of interfaces is. One could also not use inheritance at all, and provide properties to get to interfaces, but you would lose the ability to cast objects to their interfaces.

Lets take the final example in our Single Responsibility Principle tutorial and see how we can make sure that we do not expose a PrintAGraphInA4 method to more than it needs to know. I’ve stripped out the dependent classes and changed the Main() routine so that it simply tries to call a helper method to Print the graph.
Here’s the code before ISP.

   namespace ISP
   {
        class Printer
        {
            public enum PaperType { A4_PAPER, A3_PAPER };
            public PaperType Paper { get; set; }
        }

        static class Program
        {
            // small, fictional program to load up a previous graph settings file
            // and print the graph
            static void Main()
            {
                MyGraph graph = new MyGraph();

                graph.Load(@"C:\settings.txt");

                PrintAGraphInA4(graph);
            }

            static void PrintAGraphInA4(MyGraph graph)
            {
                // Note - the code only needed access to the Print Method
                // but was exposed to Load/Save/MathModel/GraphTitle
                Printer printer = new Printer();
                printer.Paper = Printer.PaperType.A4_PAPER;
                graph.Print(printer);
            }

        }

        public enum TransformationModel { none, modelX1, modelX2, modelX3 };

        class MyGraph
        {
            private MyGraphSettingsModel settings = new MyGraphSettingsModel();
            private MyGraphDataModel dataModel = new MyGraphDataModel();
            private MyGraphStorage storage = null;
            private MyGraphPrinter printer = null;
            private MyGraphPointsCalculator calc = null;

            public MyGraph()
            {
                storage = new MyGraphStorage(settings);
                printer = new MyGraphPrinter(dataModel);
                calc = new MyGraphPointsCalculator(dataModel);
            }

            public TransformationModel MathModel
            {
                get
                {
                    return settings.Model;
                }
                set
                {
                    settings.Model = value;
                    calc.CalculatePoints(settings.Model);
                }
            }

            public String GraphTitle
            {
                get { return settings.GraphTitle; }
                set { settings.GraphTitle = value; }
            }

            public void Load(String fileName)
            {
                storage.Load(fileName);
                calc.CalculatePoints(settings.Model);
            }

            public void Save(String fileName)
            {
                storage.Save(fileName);
            }

            public void Print(Printer printerDevice)
            {
                printer.Print(printerDevice);
            }
        }
   }

In the example above, the function PrintAGraphInA4 is now tightly coupled to a MyGraph class. We could have ‘solved’ that by creating a base class of Graph with overridable Print methods. However, that still leaves the PrintAGraphInA4 function exposed to a number of methods it does not need (Load/Save and more). This means we have to review PrintAGraphInA4 when MyGraph is changed. Thus creating a base class isn’t the silver bullet.

The more useful thing to do would be to create an IPrintable interface. This would allow our function to be reused by more than just graphs. It also means that ONLY changes to the IPrintable interface or the implementation of the graph’s IPrintable will be the catalyst for a code review of PrintAGraphInA4.

Here is the minimal code change:

        static class Program
        {
            // small, fictional program to load up a previous graph settings file
            // and print the graph
            static void Main()
            {
                MyGraph graph = new MyGraph();

                graph.Load(@"C:\settings.txt");

                // because inheritance is used for MyGraph : IPrintable
                // we can simply pass the graph to automatically cast to IPrintable
                PrintAnyThingPrintableInA4(graph); 
            }

            static void PrintAnyThingPrintableInA4(IPrintable printable)
            {
                // we do not care what we are printing. 
                Printer printer = new Printer();
                printer.Paper = Printer.PaperType.A4_PAPER;
                printable.Print(printer);
            }
        }

        interface IPrintable
        {
            void Print(Printer printerDevice);
        }

        class MyGraph : IPrintable
        {

We could create interfaces for IGraphStorage and IGraphSettings, but only if is beneficial.

What if I need to pass multiple interfaces to a class, should I create an aggregate?
Bob recommends not passing the aggregate interface object around if multiple interfaces are used in a client method or class. In nearly every case it would be preferable to pass the specific interfaces.

So, when should I join interfaces together?
If, for example, a class has been split into a number of interfaces to cover client usage patterns but two interfaces are always used together to deliver the full service to the client, it might make sense to combine those interfaces into one. This is especially true if both interfaces implement one or more methods in the same way and you believe there was an over zealous implementation of ISP.

Should I develop a service based model or a client based model?
A service model delivers an interface whose methods provide a distinct service to clients. We might develop such an interface in the absence of any client code. E.g. IPushNotificationService, with a simple SendNotifications() method.

A client model delivers an interface whose methods provide only what is required from empirical analysis of client usage patterns. If left unmonitored, this can introduce many client-specific interfaces tailored to specific needs of individual classes/methods. This can really help minimize the impact of change, but also can lead to inflexibility and prevent reuse.

The Middleton Rule

  • If clients of a class consistently only use a subsection of methods, it’s likely those methods should be an interface.
  • If you are passing a large object around because it’s easy – you are only storing up technical debt. Apply some form of ISP.
  • Don’t get silly. A gazillion “one method interfaces” will drive you crazy.
  • When you start to think “I shouldn’t change this class because so many others might use it and I don’t know the impact”, you’re probably exposing too much.
  • If you find fat interfaces are causing a huge dependency chain for your production code (and tests) , apply some more ISP.
  • For fat 3rd party interfaces, create your own Adapter in the middle and separate interfaces.

References

http://www.oodesign.com/interface-segregation-principle.html

http://www.objectmentor.com/resources/articles/isp.pdf

Pragmatic SOLID Part 2 – The Open/Closed Principle

Software entities (classes, modules, functions etc.)  should be open for extension but closed for modification- Robert C. Martin February 1997

TL;DR

  • Let the act of changing code be the catalyst for protecting against that kind of change in future.
  • Not every change can be protected against, but the action of closing methods allows easy extension later.
  • Use programming features (such as abstraction and interfaces) to enable addition or removal of leaf classes without impacting existing code.
  • Avoid distributing RunTime Type Information across your code-base, it makes your code brittle.
  • Classes within a derivation should know nothing of other concrete classes in that derivation – Try and keep ‘set’ logic away from consuming code and concretions.
  • And remember – “Resisting premature abstraction is as important as abstraction itself” Robert C Martin.

What’s the business value?

The ideal outcome of applying the Open/Closed Principle (OCP) is to reduce costs through designing classes and interfaces which withstand a degree of expected change, such that it will have little impact on existing code. This can reduce consequential errors and increase future velocity. Protecting against changes which never materialize is a poor return on investment.

Come again?

The Open aspect of the principle means preparing code to be extensible in future. That could be through abstraction and extracting type-detection logic from consuming classes

E.g. Code immediately takes advantage of a new leaf class in an existing class hierarchy without impacting existing code.

The Closed aspect of the principle means protecting code from the risks of change.

E.g. Adding or removing a leaf class in an existing class hierarchy has a low impact on existing code.

This is simply applying principles of polymorphism, encapsulation, information hiding and abstraction appropriately.

Naturally, if you add a method to a class it is likely to require review and modification of previously Closed code. While you are making those changes ask yourself “if I make this kind of change again, can I protect against having to do so much work?”. That’s the attitude the principle is trying to promote.

Show me an example

Example 1 – Common OCP application
This example uses the concept of a Graph and Printer class from my previous post – The Single Responsibility Principle, but ignores the Single Responsibility Principle for a while.

Pretend a new junior programmer called Bob is introducing a new printer class for a 3d printer, to be used by the MyGraph class. Being a complete idiot, Bob passes in two printer classes and conditionally calls print on each as the code below shows.

namespace OCP1
{
    class Printer2d
    {
        public void Print(){ }
    }

    class Printer3d
    {
        public void Print() { }
    }

    class MyGraph
    {
        public MyGraph() { ; }
        public void Print(Printer2d printer2d, Printer3d printer3d)
        {
            if (printer2d != null)
            {
                printer2d.Print();
            }
            else if (printer3d != null)
            {
                printer3d.Print();
            }
            else
            {
                // must be a new type of printer
            }
        }
    }

    class TestClass
    {
        public void Test()
        {
            MyGraph graph = new MyGraph();
            graph.Print(null, new Printer3d());
        }
    }
}

Bob’s co-worker, Dave, slaps Bob around the head and tells him to create an abstract base class. He also asks Bob “What happens when we pass both printers, do you really intend to print twice?”. Bob’s review might be awkward if he continues to be an idiot. By introducing an abstract base class (or an interface) we’ve closed off the Print function to that type of change in future. Here’s how that looks.

namespace OCP2
{
    abstract class Printer
    {
        public abstract void Print();
    }

    class Printer2d : Printer
    {
        override public void Print() { }
    }

    class Printer3d : Printer
    {
        override public void Print() { }
    }

    class MyGraph
    {
        public MyGraph() { ; }
        public void Print(Printer anyPrinter)
        {
            anyPrinter.Print();
        }
    }

    class TestClass
    {
        public void Test()
        {
            MyGraph graph = new MyGraph();
            graph.Print(new Printer3d());
            graph.Print(new Printer2d());
        }
    }
}

This example is trivial, in the real world there’s likely to be some property one printer supports that the other does not. Imagine the 3d Printer has a SetDepth Function, which moves coordinates further down the Z order. This is useful for aesthetics when printed. Let’s see how Bob manages this change.

namespace OCP3
{
    abstract class Printer
    {
        public abstract void Print();
    }

    class Printer2d : Printer
    {
        override public void Print() { } 
    }

    class Printer3d : Printer
    {
        override public void Print() { }
        public void SetDepth(int depth) { }
    }

    class MyGraph
    {
        public MyGraph() { ; }
        public void Print(Printer anyPrinter)
        {
            if (anyPrinter is Printer3d)
            {
                Printer3d printer3d = (Printer3d)anyPrinter;
                printer3d.SetDepth(2);
            }
            anyPrinter.Print();
        }
    }

    class TestClass
    {
        public void Test()
        {
            MyGraph graph = new MyGraph();
            graph.Print(new Printer3d());
            graph.Print(new Printer2d());
        }
    }
}

Well, Bob’s implementation is not wrong. If that is the only code in the whole system dealing with printers and graphs it will be fine to leave it alone and move on. But if we know we will be adding or removing a printer, especially the 3d printer, this code is too rigid. If you can avoid branching logic based on type, your code will be more closed to change and open to extension.
Let’s imagine there’s a 4d Printer, which also supports the depth method.

namespace OCP4
{
    abstract class Printer
    {
        public abstract void Print();
    }

    class Printer2d : Printer
    {
        override public void Print() { }
    }

    class Printer3d : Printer
    {
        override public void Print() { }
        public void SetDepth(int depth) { }
    }

    class Printer4d : Printer
    {
        override public void Print() { }
        public void SetDepth(int depth) { }
    }

    class MyGraph
    {
        public MyGraph() { ; }
        public void Print(Printer anyPrinter)
        {
            if (anyPrinter is Printer3d)
            {
                Printer3d printer3d = (Printer3d)anyPrinter;
                printer3d.SetDepth(2);
            }
            if (anyPrinter is Printer4d)
            {
                Printer4d printer4d = (Printer4d)anyPrinter;
                printer4d.SetDepth(2);
            }
            anyPrinter.Print();
        }
    }

    class TestClass
    {
        public void Test()
        {
            MyGraph graph = new MyGraph();
            graph.Print(new Printer3d());
            graph.Print(new Printer2d());
            graph.Print(new Printer4d());
        }
    }
}

We are now building logic into every graph, for every printer. Madness. So how do we close the MyGraph.Print method from the types of changes we’ve experienced so far? The OCP does not dictate how you achieve this, but just that you should try and achieve it.

Use an interface to represent capability
🙂 Keeps base class clean
🙂 Only derivations expose the functionality if truly supported
🙂 Callers can’t use methods without casting the interface. This means it is less error prone for consumers who might not realize this is not supported on all printers.
😦 Unless you maintain good documentation in your base class comments, a new derivation doesn’t know what interfaces could be implemented.
My View: The preferred approach. It keeps the code to a minimum, makes it obvious that this is specific functionality only a set of classes support and is unambiguous.

namespace OCPUsingInterfaces
{
    abstract class Printer
    {
        public abstract void Print(); 
    }

    interface IPrinterDepthCapable
    {
        void SetDepth(int depth);
    }

    class Printer2d : Printer
    {
        public override void Print() { }
    }

    class Printer3d : Printer, IPrinterDepthCapable
    {
        public override void Print() { }

        void IPrinterDepthCapable.SetDepth(int depth) { }
    }

    class Printer4d : Printer, IPrinterDepthCapable
    {
        public override void Print() { }

        void IPrinterDepthCapable.SetDepth(int depth) { }
    }

    class MyGraph
    {
        public MyGraph() { ; }
        public void Print(Printer anyPrinter)
        {
            // it's obvious SetDepth is only supported by some classes
            if( anyPrinter is IPrinterDepthCapable)
                ((IPrinterDepthCapable)anyPrinter).SetDepth(2);

            anyPrinter.Print();
        }
    }

    class TestClass
    {
        public void Test()
        {
            MyGraph graph = new MyGraph();
            graph.Print(new Printer3d());
            graph.Print(new Printer2d());
            graph.Print(new Printer4d());
        }
    }
}

🙂 In the example above we have demonstrated how to protected the Print() function from addition or removal of similar printer classes.
🙂 We’ve also protected the Print() function from classes which do or do not implement SetDepth() without complicating the base class interface.
🙂 No concrete implementation of a Graph knows about any concrete implementation of a Printer class.
😦 We haven’t truly protected against adding another capability or another function to the IPrinterDepthCapable interface. But it’s not possible to protect classes against every change.
😦 We still have to search across all Graph classes to determine how they may be adapted if an interface is added.

To improve closure of the Print() function further we could apply the Single Responsibility Principle and move the printing logic into a MyGraphPrinter class. This can just move the problem elsewhere, but would ensure that a change to printing logic only affected classes like MyGraphPrinter only, and not MyGraph.

Example 2- Where to store relational logic
Sometimes features force developers to develop meta-knowledge about concretions, to rank one class over another. For a quality such as speed of printing, the implementation is trivial – a property on each printer using an abstract base class member will do. The code will remain nicely closed.

But if the quality is relative to other concretions it is much more likely that a poor implementation will result. For example, how do we choose the most capable printer installed on the system? We know a 4d Printer is more capable than a 3d printer and that is more capable than 2d printer. Where does that code live?

It’s been a few weeks since Bob has shown his mentor Dave some code, let’s see if Bob has some new tricks.

namespace OCPMostDesirablePrinter
{
    abstract class Printer : IComparable
    {
        public abstract void Print();
        public abstract bool IsInstalled { get; }
        public abstract int CompareTo(object o);
    }

    interface IPrinterDepthCapability
    {
        void SetDepth(int depth);
    }

    class Printer2d : Printer
    {
        public override void Print() { }
        public override bool IsInstalled { get { return true; } }// hard coded for test purposes
        public override int CompareTo(object o)
        {
            if (o is Printer3d)
                return 1; // 2d should come after 3d
            if (o is Printer4d)
                return 1; // 2d should come after 4d
            return 0;
        }
    }

    class Printer3d : Printer, IPrinterDepthCapability
    {
        public override void Print() { }

        void IPrinterDepthCapability.SetDepth(int depth) { }
        public override bool IsInstalled { get { return true; } }// hard coded for test purposes
        public override int CompareTo(object o)
        {
            if (o is Printer2d)
                return -1; // 3d should come before 2d
            if (o is Printer4d)
                return 1; // 3d should come after 4d 
            return 0;
        }
    }

    class Printer4d : Printer, IPrinterDepthCapability
    {
        public override void Print() { }

        void IPrinterDepthCapability.SetDepth(int depth) { }
        public override bool IsInstalled { get { return false; } } // hard coded for test purposes
        public override int CompareTo(object o)
        {
            if (o is Printer2d)
                return -1; // 4d should come before 2d
            if (o is Printer3d)
                return -1; // 4d should come before 3d
            return 0;
        }
    }

    class MyGraph
    {
        public MyGraph() { ; }
        public void Print(Printer anyPrinter)
        {
            // it's obvious SetDepth might not work for all
            if (anyPrinter is IPrinterDepthCapability)
                ((IPrinterDepthCapability)anyPrinter).SetDepth(2);

            anyPrinter.Print();
        }
    }

    class TestClass
    {
        public void Test()
        {
            List allPrinters = new List(){ new Printer2d(), new Printer3d(), new Printer4d() };

            List installedPrinters = allPrinters.Where(x => x.IsInstalled).ToList();

            if (installedPrinters.Count > 0)
            {
                installedPrinters.Sort();

                Printer bestPrinter = installedPrinters[0];

                MyGraph graph = new MyGraph();
                graph.Print(bestPrinter);
                graph.Print(bestPrinter);
                graph.Print(bestPrinter);
            }            
        }
    }
}

Wow. Bob has learned some new tricks. List initialization, lambda expressions. He’s getting some snazzy skills there. However…
🙂 Using sort was quite clever, but what does that really mean to a consumer, they might expect it sorts by paper available, speed, name of printer.
😦 Bob has introduced a terrible burden on each concrete printer class – every printer class has knowledge of every printer class.
😦 The CompareTo() indicates what should come first in the list, not really what’s ‘greater’ than each other
😦 What does it really mean to compare a printer against each other – it doesn’t even make sense.
😦 It feels like we have copied and pasted code three times. Something is wrong!

Dave resorts to more head slapping, and nearly knocks Bob unconscious. It’s a brutal world.
Here’s what they came up with to make sure that client code was not exposed to the printers concrete classes, the printers do not know about each other, and that the code could be read easily. This code is closed to change and open to extension.

namespace OCPMostDesirablePrinterFactory
{
    abstract class Printer
    {
        public abstract void Print();
        public abstract bool IsInstalled { get; }
    }

    interface IPrinterDepthCapability
    {
        void SetDepth(int depth);
    }

    class Printer2d : Printer
    {
        public override void Print() { }
        public override bool IsInstalled { get { return true; } }// hard coded for test purposes
    }

    class Printer3d : Printer, IPrinterDepthCapability
    {
        public override void Print() { }

        void IPrinterDepthCapability.SetDepth(int depth) { }
        public override bool IsInstalled { get { return true; } }// hard coded for test purposes
    }

    class Printer4d : Printer, IPrinterDepthCapability
    {
        public override void Print() { }

        void IPrinterDepthCapability.SetDepth(int depth) { }
        public override bool IsInstalled { get { return false; } } // hard coded for test purposes
    }

    class MyGraph
    {
        public MyGraph() { ; }
        public void Print(Printer anyPrinter)
        {
            // it's obvious SetDepth might not work for all
            if (anyPrinter is IPrinterDepthCapability)
                ((IPrinterDepthCapability)anyPrinter).SetDepth(2);

            anyPrinter.Print();
        }
    }

    class PrinterFactory
    {
        public static Printer FindMostDesirablePrinterInstalled()
        {
            // keep this list in order of most desirable first
            List allPrinters = new List(){
                new Printer4d(),
                new Printer3d(),
                new Printer2d() };

            return allPrinters.FirstOrDefault(x => x.IsInstalled);
        }
    }

    class TestClass
    {
        public void Test()
        {
            Printer bestPrinter = PrinterFactory.FindMostDesirablePrinterInstalled();

            if (bestPrinter!=null)
            {
                MyGraph graph = new MyGraph();
                graph.Print(bestPrinter);
                graph.Print(bestPrinter);
                graph.Print(bestPrinter);
            }
        }            
    }
}

🙂 The code is much much much cleaner.

🙂 The ordering logic of desirable printers is trivial to maintain.

🙂 The the logic is easy to locate.

When should I retrospectively apply the principle to a class?

There will always be a change that a design is not closed to, thus the application of the OCP needs to be strategic. The designer must predict the most probable changes the system will endure, or react to them as they happen. The time taken to refactor and design code to meet OCP can be expensive. Over-enthusiastic application of OCP can increase maintenance costs through increased code complexity. Thus, choosing the proportionate amount of effort to expend is important.

Robert C. Martin talks about taking ‘the bullet’ when the first change request comes in – letting that change request show you the likely pattern of change. When a bullet comes in, make your code bullet proof to those kinds of bullets.

The Middleton test – to maximise ROI:

  • If you are changing some code, try and protect the code from this type of change if you can.
  • If you have doubts about the forthcoming changes materializing, do not do the work now.

References

Full Article –  Robert C. Martin

Wikipedia

Further examples for Example 1 – Common OCP application

The Interface pattern is not the only way of solving the problem in Example 1. Languages such as C# and C++ give us many different ropes to hang ourselves with. Each has pros and cons, and some are just plain stupid to even try.

Inject an intermediate abstract class
🙂 can test for the capability rather than the object.
🙂 It is now immune (Closed) to other printers which may or may not implement this capability.
😦 You have to choose unique method names for each abstract class you implement.
😦 If we add another capability to printers, where does it sit in the inheritance hierarchy?
😦 This is an abuse of inheritance – Is this really a ‘type of’ graph? Your colleagues won’t respect you!

My view: I would not recommend this approach.

namespace OCPIntermediateAbstractBaseClass
{
    abstract class Printer
    {
        public abstract void Print();
    }

    abstract class PrinterWhichSupportsDepth : Printer
    {
        public abstract void SetDepth(int depth);
    }

    class Printer2d : Printer
    {
        override public void Print() { }
    }

    class Printer3d : PrinterWhichSupportsDepth
    {
        override public void Print() { }
        override public void SetDepth(int depth) { }
    }

    class Printer4d : PrinterWhichSupportsDepth
    {
        override public void Print() { }
        override public void SetDepth(int depth) { }
    }

    class MyGraph
    {
        public MyGraph() { ; }
        public void Print(Printer anyPrinter)
        {
            // much simpler than before
            if (anyPrinter is PrinterWhichSupportsDepth)
            {
                PrinterWhichSupportsDepth printerDepth = (PrinterWhichSupportsDepth)anyPrinter;
                printerDepth.SetDepth(2);
            }
            anyPrinter.Print();
        }
    }

    class TestClass
    {
        public void Test()
        {
            MyGraph graph = new MyGraph();
            graph.Print(new Printer3d());
            graph.Print(new Printer2d());
            graph.Print(new Printer4d());
        }
    }
}

Use multiple inheritance
😦 This isn’t supported in C#, but is in C++.
😐 Is not a terrible approach in C++ (non .NET) if you do not want to introduce COM Interfaces. It would require a spin doctor to call these ‘is a’ relationships.

Provide a SetDepth method in the base class which is ignored by some.
🙂 Can call without worry.
🙂 Minimal code
🙂 Smaller public interface
😦 Callers might expect the method to do something
😦 Every derivation has to implement something they may never use. If you do not make it abstract in the base class, derivatives have to guess that they should implement the function.
My view: Not so terrible, and quite a common thing you’ll see – but that doesn’t make it right.

namespace OCPSetDepthInBaseClass
{
    abstract class Printer
    {
        public abstract void Print();
        public abstract void SetDepth(int depth);
    }

    class Printer2d : Printer
    {
        override public void Print() { }
        // really does nothing, I have to implement this because of another printer 😦
        override public void SetDepth(int depth) { }
    }

    class Printer3d : Printer
    {
        override public void Print() { }
        override public void SetDepth(int depth) { }
    }

    class Printer4d : Printer
    {
        override public void Print() { }
        override public void SetDepth(int depth) { }
    }

    class MyGraph
    {
        public MyGraph() { ; }
        public void Print(Printer anyPrinter)
        {
            // MUCH SIMPLER!!!
            anyPrinter.SetDepth(2);
            anyPrinter.Print();
        }
    }

    class TestClass
    {
        public void Test()
        {
            MyGraph graph = new MyGraph();
            graph.Print(new Printer3d());
            graph.Print(new Printer2d());
            graph.Print(new Printer4d());
        }
    }
}

Introduce a capability method, and make SetDepth non abstract
🙂 We know that when we call SetDepth that it will do something.
🙂 Derivatives do not have to implement SetDepth() when it makes no sense.
🙂 Derivatives have to implement IsSetDepthSupported()
🙂 Code is simple and predictable
😦 Two methods are exposed in the base class
😦 Public interface gets bigger per capability
😦 Callers have to know to call IsSetDepthSupported() before SetDepth() – error prone.
My View: Maybe suitable for smaller sets of functionality, the benefit is that it does make the capability appear optional and available without hiding capabilities as interfaces.

namespace OCPIsDepthSupportedCapability
{
    abstract class Printer
    {
        public abstract void Print();

        public abstract bool IsDepthSupported();
        public virtual void SetDepth(int depth) {  /* do nothing by default*/ }
    }

    class Printer2d : Printer
    {
        public override void Print() { }
        public override bool IsDepthSupported() { return false; }

        // don't have to implement SetDepth
    }

    class Printer3d : Printer
    {
        public override void Print() { }

        public override bool IsDepthSupported() { return true; }
        public override void SetDepth(int depth) { }
    }

    class Printer4d : Printer
    {
        public override void Print() { }

        public override bool IsDepthSupported() { return true; }
        public override void SetDepth(int depth) { }
    }

    class MyGraph
    {
        public MyGraph() { ; }
        public void Print(Printer anyPrinter)
        {
            // it's obvious SetDepth might not work for all
            if( anyPrinter.IsDepthSupported() )
                anyPrinter.SetDepth(2);

            anyPrinter.Print();
        }
    }

    class TestClass
    {
        public void Test()
        {
            MyGraph graph = new MyGraph();
            graph.Print(new Printer3d());
            graph.Print(new Printer2d());
            graph.Print(new Printer4d());
        }
    }
}

(C) Giles Middleton  2013

Pragmatic SOLID Part 1 – The Single Responsibility Principle

There should never be more than one reason for a class to change. – Robert C. Martin February 1997

TL;DR

  • Break up your classes into teeny tiny ones.
  • You’re aiming to limit the impact and complexity of change within a class.
  • Don’t use brainlessly – there needs to be a symptom or good reason.

What’s the business value?

The ideal outcome of applying the Single Responsibility Principle (SRP) is to reduce assessment, development and testing time when making changes to a class in future. We like that, right? We could even use that in a discussion with Product Owners around reducing technical debt! Wait, it does more than that. The principle consequently effects an improved flexibility and reuse pattern through a class which is simpler, smaller and less prone to consequential defects. Each resulting class starts to become more cohesive and testable.

This is achieved by decomposing your (lovely) encapsulated class by separating concerns within the class, into smaller classes.

A class’ data, calculation capability, save/load capability and print capability result in four classes. You then have ultimate confidence what the impact of your change will be. Some would even go so far as to split save from load.

It’s not all good news though. Some people quote the SRP to say “this class does one thing and one thing only, really well”, I think that’s a misinterpretation. Applying the principle unnecessarily can quickly lead to increased complexity and confusion on large projects due to high volumes of small, poorly named single-method classes which would have previously been encapsulated within one class.  Good naming conventions, and feature grouping can help with this, but why are you now fighting with your code? Something went wrong.

Show me an example!

Here’s an example which demonstrates the SRP applied to a class which encapsulates a graph. The points are calculated based on a transformation model. The graph can persist and restore its settings on disk, and provides the capability to print given a Printer class.

namespace MyGraphProgramV1
{
    // dummy class
    class Printer
    {
    }

    static class Program
    {
        // small, fictional program to load up a previous graph settings file
        // print the old version, update the title and mathematical model
        // and resave the settings
        static void Main()
        {

            MyGraph graph = new MyGraph();

            graph.Load(@"C:\settings.txt");

            Printer printer = new Printer();
            graph.Print(printer);

            graph.GraphTitle = "NewTitle";
            graph.MathModel = TransformationModel.modelX2;

            graph.Save(@"C:\settings.txt");
        }
    }

    public enum TransformationModel { none, modelX1, modelX2, modelX3 };

    class MyGraph
    {
        // version 1 - multiple reasons to change
        // 1. If a setting changes
        // 2. if the printing method changes
        // 3. If the calculation engine changes
        // 4. If the storage mechanics change
        // 5. If the point array changes
        // the class as a whole is at risk when each of these change. 
        // imagine if each function was a few hundred lines. 
        // This would start to become difficult to read.

        public MyGraph() { ; }

        public String GraphTitle
        {
            set
            {
                graphTitle = value;
            }
            get
            {
                return graphTitle;
            }
        }

        public TransformationModel MathModel
        {
            get { return transformationModel; }
            set
            {
                transformationModel = value;
                CalculatePoints();
            }
        }

        public void Load(String fileName)
        {
            // load title and transformation model, probably from an xml file
            // possibly upgrade the settings in future.
            CalculatePoints();
        }

        public void Save(String fileName)
        {
            // save title and transformation model, handle file exceptions. 
        }

        public void Print(Printer printer)
        {
            // print title and points, draw some axis, make it look pretty.
            // this function could get quite large and use a lot of common classes.
        }

        private void CalculatePoints()
        {
            // depending on transformationModel, 
            // populate points with a mathematical algorithm
            // this will probably be a large switch statement to start with.
        }

        private int[,] points = new int[10, 10];
        private TransformationModel transformationModel;
        private String graphTitle;
    }
}

As you will see with the Single Responsibility Principle applied below, some classes still have more
than one reason for change. You can chase down the rabbit hole but that can get silly. You have to ask yourself where do you require reduced complexity, clarity, reuse, testability. The code below reduces the impact of a change on the entirety of the MyGraph class.

namespace MyGraphProgramV2
{
    // dummy class
    class Printer
    {
    }

    static class Program
    {
        // small, fictional program to load up a previous graph settings file
        // print the old version, update the title and mathematical model
        // and re-save the settings
        static void Main()
        {
            // version 2 Single Responsibility Principle implemented 
            // Note: there are no changes to our client!.
            MyGraph graph = new MyGraph();

            graph.Load(@"C:\settings.txt");

            Printer printer = new Printer();
            graph.Print(printer);

            graph.GraphTitle = "NewTitle";
            graph.MathModel = TransformationModel.modelX2;

            graph.Save(@"C:\settings.txt");
        }
    }

    public enum TransformationModel {none, modelX1, modelX2, modelX3};

    class MyGraph
    {
        // version 2 - Single responsibility principle applied
        // This class' responsibility is now to orchestrate and 
        // route data between SRP classes.
        // It's much easier to read. Reasons for change:
        //    1. If classes require more data or different interfaces
        //    2. If we add more properties we have to route them on to the settings.

        // We could create these classes when needed, most don't need to be members. #todo
        private MyGraphSettingsModel settings = new MyGraphSettingsModel();
        private MyGraphDataModel dataModel = new MyGraphDataModel();
        private MyGraphStorage storage = null;
        private MyGraphPrinter printer = null;
        private MyGraphPointsCalculator calc = null;

        public MyGraph()
        {
            storage = new MyGraphStorage(settings);
            printer = new MyGraphPrinter(dataModel);
            calc = new MyGraphPointsCalculator(dataModel);
        }

        public TransformationModel MathModel
        {
            get 
            { 
                return settings.Model; 
            }
            set
            {
                settings.Model = value;
                calc.CalculatePoints(settings.Model);
            }
        }

        public String GraphTitle 
        {
            get { return settings.GraphTitle; }
            set { settings.GraphTitle = value; }
        }

        public void Load(String fileName)
        {
            storage.Load(fileName);
            calc.CalculatePoints(settings.Model); // need to calculate points after a load
        }

        public void Save(String fileName)
        {
            storage.Save(fileName);
        }

        public void Print(Printer printerDevice)
        {
            printer.Print(printerDevice);
        }
    }

    class MyGraphSettingsModel
    {
        // This class' reasons for change: 
        // 1. If a property is added or removed from the settings class

        private TransformationModel model;
        private String graphTitle;

        public TransformationModel Model
        {
            get { return model; }
            set { model = value; }
        }

        public String GraphTitle
        {
            get { return graphTitle; }
            set { graphTitle = value; }
        }
    }

    class MyGraphDataModel
    {
        // This class' reasons for change: 
        //    1. If the data points model changes 

        public int[,] points = new int[10,10];
    }

    class MyGraphStorage
    {
        // This class' reasons for change:
        //  1. if the settings model changes 
        //     - this class could get busy if we need to perform 
        //       upgrades on older versions when loading.
        //     - by its own merit that's a great reason to move 
        //       it away from the main class
        //  2. If we change from file storage to internet/memory 
        //     - so we could separate further through interfaces.
        // 
       private MyGraphSettingsModel settings = null;

       public MyGraphStorage(MyGraphSettingsModel model)
       {
           settings = model;
       }

       public void Save(String fileName) { return; } // does nothing for now
       public void Load(String fileName) { return; } // does nothing for now
    }

    class MyGraphPrinter
    {
        // This class' reasons for change
        // 1. If the data model changes, we have to update the 
        //    printing routine - we could pass explicit properties instead
        // 2. if the printer API/interface changes
        private MyGraphDataModel dataModel = null;
        public MyGraphPrinter(MyGraphDataModel model)
        {
            dataModel = model;
        }
        public void Print(Printer printer) { }
    }

    class MyGraphPointsCalculator
    {
        // This class' reasons for change
        // 1. The implementation of the calculation engine changes
        // 2. a new calculation is required.
        MyGraphDataModel dataModel = null;
        public MyGraphPointsCalculator(MyGraphDataModel model) 
        { dataModel = model; }
        public void CalculatePoints(TransformationModel model) 
        { ; } // update points
    }
}

When should I retrospectively apply the principle to a class?

In Robert Martin’s Book (Agile principles, patterns and practices in C#), he warns against applying the Single Responsibility Principle to introduce needless complexity. Ask yourself if you have skim read over that line. At times, I certainly forget that!

The Middleton test – to maximise ROI:

  • If unexpected defects are regularly introduced when this class is altered, decouple concerns, apply the principle to the class.
  • If you are planning on duplicating part of the functionality of this class, apply the principle to that part.
  • If you require future flexibility for varying implementations (E.g. calculation engines, drawing routines, printing routines, saving to different media), then apply the principle in anticipation of this.
  • If none of the above are true, and you have more valuable things to be undertaking – do them instead.

When exercising the principle, your newly extracted classes are ripe candidates for a strong set of supporting tests. This is especially important if the extracted classes are still complex internally and you plan to reuse it.

As you can tell I’m not an advocate of hyper-proactive application of SRP. I’m not alone. My bold assertion is – that If you are confident that your class is not going to be subject to change, doesn’t exhibit a set of bugs which flip-flop regularly and its code offers no future re-use, then it probably isn’t cost effective to apply the SRP to it.

So what is the official description of the Single Responsibility Principle?

Robert C Martin (Uncle Bob). Feb 1997
“In the context of the Single Responsibility Principle (SRP) we define a responsibility to be “a reason for change.” If you can think of more than one motive for changing a class, then that class has more than one responsibility.”
“If a class has more then one responsibility, then the responsibilities become coupled. Changes to one responsibility may impair or inhibit the class’ ability to meet the others. This kind of coupling leads to fragile designs that break in unexpected ways when changed”

The typical examples given across the web are:

  • a class that not only holds the document data, but saves and prints it too.
  • a rectangle (or other geometric) class that has an area calculation function and a drawing function.

References

Full description (Chapter Excerpt) from Robert C. Martin

Developer Express video on SRP

Wikipedia

Curly’s Law – Jeff Atwood

OODesign.com

Bob Martin explaining the SRP

And, just google. There’s lots of great examples and viewpoints.

(C) Giles Middleton  2013

Moqing quick reference (Cheat Sheet)

This post is just a quick reference for myself, when I’ve spent a while away from TDD and want to remember some of the tricks.

Initial construction

var mockFoo = new Mock<IFoo>(MockBehavior.Strict); 

Accessing the mock object as IFoo

IFoo foo = mockFoo.Object;

Initializing the inversion of control container (StructureMap)

Generally use Configure, not Initialize. Configure is additive, Initialize is not. Here’s a complete test method showing the different ways we typically initialize.

[TestMethod()]
public void TestMethod()
{
   var mockOrange = new Mock<IOrange>(MockBehavior.Strict);
   MyDefaultHandlerForPear pearConcretion = new MyDefaultHandlerForPear();
   ObjectFactory.Configure( x=> 
   {
      // use mock object
     x.For<IOrange>().Use(mockOrange.Object); 
     // use new class when asked
     x.For<IApple>().Use<MyDefaultHandlerForApple>();
     // use an existing instance that implements IPear 
     x.For<IPear>().Use(pearConcretion);

    });
}

Setting up void calls

public interface IOrange
{
  void Peel();
}
[TestMethod()]
public void TestOrange()
{
  var mockOrange = new Mock<IOrange>(MockBehavior.Strict);
  mockOrange.Setup(x => x.Peel());
}

Setting up simple return responses

public interface IOrange { int CountSlices(); }
...
var mockOrange = new Mock<IOrange>(MockBehavior.Strict);
mockOrange.Setup(x => x.CountSlices()).Returns(5);

Setting up open parameterized calls

When methods have parameters, you can allow calls to be made regardless of if the parameter’s values are rubbish by using the convention It.IsAny<T>().

This becomes more meaningful when you use MockBehaviour.Strict, as you must call Setup for all functions that are going to be called, even if you’re not really interested if they work.

public interface IOrange { int CountSlices(bool containingPips); }
...
 mockOrange = new Mock<IOrange>(MockBehavior.Strict);
 mockOrange.Setup(x => x.CountSlices(It.IsAny<bool>())).Returns(5);

Setting up closed calls

If you want to ensure only specific parameter values or objects are passed, your setup can explicitly allow these. From the above snippet..

mockOrange.Setup(x => x.CountSlices(false)).Returns(2);
mockOrange.Setup(x => x.CountSlices(true)).Returns(3);

Setting up a function callback

If you want to just call a function, use the anonymous lambda within the return statement.

mockOrange.Setup(x => x.CountSlices(false)).Returns(()=>{ CallMyFunction(); return 2;} );

Setting up intelligent behaviour using parameters

Extending the above syntax, if your goal is to stimulate your class under test with conditional behaviours, you can provide fake implementations inline. This is achieved by re-specifying the functions parameters in the lambda expression.

public interface IOrange { int CountSlices(bool containingPips, int juicePercentage); }
[TestMethod()]
public void TestOrange()
{
  var mockOrange = new Mock<IOrange>(MockBehavior.Strict);
  mockOrange.Setup(x => x.CountSlices( It.IsAny<bool>(), It.IsAny<int>()))
.Returns( ( bool containingPips, int juicePercentage) => { if( containingPips && juicePercentage>50 ) return 2; else return 4; } ); }

Private method calling on classes

If you’ve got yourself into a position where you need to call a private method of a class under test, you can do so, without making them public.

My opinion – if you’re doing this, you’ve not architected your classes correctly (single responsibility principle).

See this stack exchange post on how to achieve this.

Verifying frequency of calls

public interface IOrange { int CountSlices(bool containingPips); }
[TestMethod()]
public void TestOrange()
{
   var mockOrange = new Mock<IOrange>(MockBehavior.Strict);
   mockOrange.Setup(x => x.CountSlices( It.IsAny<bool>())).Returns( 4 );
   // do something that would cause CountSlices to be called
   // verify Count slices called once with any bool
   mockOrange.Verify(x => x.CountSlices(It.IsAny<bool>()), Times.Once());
   // verify Count slices called twice with true
   mockOrange.Verify(x => x.CountSlices(true), Times.Exactly(2));
   // verify Count slices never called with false
   mockOrange.Verify(x => x.CountSlices(false), Times.Never());
}