DEV Community

zakwillis
zakwillis

Posted on

An awesome way to avoid violating the open closed principle

One of those days where I think, I wonder if I can do this?

You know when you have been working on a big piece of work, and know that tying up the loose ends will be painful to write. Well, I faced that this morning. I often store Actions/objects in a list and pass them in. A quick example is something like this.

interface IValidator
{
bool IsValid(SomeObject something);
}

IList<IValidator> validators = new List<Validator>()
{{new GoodlookingEnough()},{new SmartEnough()},{new StinkyFeetTest()},{new TooGoodForMeTest()}};

We can then inject those into a class and validate objects without having to add lots of tests inside the class. This is a perfect way of avoiding the open closed principle. This is pseudo code, so it may be wrong.

var valid = validators.Where(x=>x.IsValid() = true);

SOLID Development

If you have an interview, we get asked - what is SOLID.
S = Single Responsibility Principle.
O = Open Closed Principle
L = Liskov Substitution Principle
I = Interface Segregation Principle
D = Dependency Injection Principle

All I know is, as you get better at coding, you tend to;

  • Use interfaces.
  • Use Dependency Injection.
  • Not make classes too humongous.
  • Question if you do too much switch/case statements.

About me

I have been out of contract for just over a year - out of painful choice. This has been to focus on getting my property platform out - findigl and to get my limited company up to scratch. Am nearing getting these to a point where I can either find a contract or explore new areas.
You can find out more about these projects;

The Open Closed Principle

In simple terms. If you have a class, which has selector approach, whereby it decides which version of a class to execute, you have violated the Open Closed Principle. Here is a really obvious example - ignore the hard coding.

public void(ExecuteMode execMode)
if (ExecuteMode == "Dothis")
{ 
new DoThis().Execute();
}
if (ExecuteMode == "DoSomethingElse")
{ 
new DoSomethingElse().Execute();
}

Now, imagine we want a new execution mode.
We would now add;

public void(ExecuteMode execMode)
if (ExecuteMode == "Dothis")
{ 
new DoThis().Execute();
}
if (ExecuteMode == "DoSomethingDifferent")
{ 
new DoSomethingDifferent().Execute();
}

We end up with, what is known as Jenga development, keep adding new lines of code to a class as new functionality is added. There are times when OpenClosed is fine - when mapping from a known or very static set. If we were mapping SQL Data Types to C# data types, why would we care.

Turbo charging Open Closed principle avoidance

So, I have had to spend around a week and a half (over the long Xmas period) building an application to manage deployment of applications and configuration - there will be a blog post on this application suite in due course. It is not fully tested yet, and contains a lot of functionality. The entire application can be described by the following enumeration

    public enum ProcessMode
    {
        NonExecutablesToDevelopment
            , DummyMergeConfig
            , ApplicationConfigToDevopArea
            ,DeployApplicationAndConfig
            ,Unknown
    }

As the application is a Console application, we will do the following;

  • Accept a commandline argument which maps to the Process Mode.
  • Accept an override argument.

I want to avoid having a class, somewhere in the application which does a case statement on these modes and I have to keep extending the same class.

Interfaces to the rescue.

In my Step namespace, I have a class which performs each of the process modes. These classes are bound to their own respective interface. Here is an example.

    public interface IConfigRestore : IProcessMode
    {
        IArchiver archiver { get; set; }
        ICheckFileExists checkFileExists { get; set; }
        ICheckFileIsEmpty checkFileIsEmpty { get; set; }
        IClearFile clearFile { get; set; }
        ICreateFolder createFolder { get; set; }
        IFileCopier fileCopier { get; set; }
        IGenerateExecutableArtifactPopulationSource generateExecutableArtifactPopulationSource { get; set; }
        ISetting setting { get; set; }

        void Generate();
    }

The trick, is I have another interface called IProcessMode, which IConfigRestore implements. What is interesting is, it is the concrete class which has to implement IProcessMode.

    public interface IProcessMode
    {
        ProcessMode processMode{ get; }
        void Process();

    }

The concrete implementation - ConfigRestore is where the magic happens. We implement all contracts. Here is a subset

    public class ConfigRestore : IConfigRestore
    {
            public ProcessMode processMode => ProcessMode.ApplicationConfigToDevopArea;
        public void Generate()
        {
        // do stuff...
        }
        public void Process()
        {
            Generate();
        }
}

What we have done, is let the IProcessMode implementation call the normal functionality of the ConfigRestore class. I think this probably breaks the Interface Segregation Rule, but it is definitely worth it.

I use Ninject in .Net Framework. So we can assume that these contracts have been bound to their concrete implementations.

Our commandline args...

"Unknown" "-force"

Here is my program class.

namespace IRFullDeploy
{
    class Program
    {
        static void Main(string[] args)
        {

            DI.Binder binder = new DI.Binder() { args = args };
            var kernelConfig = new KernelConfiguration(binder);

            var roKernel = kernelConfig.BuildReadonlyKernel();


            IList<IProcessMode> processModes = new List<IProcessMode>();

            processModes.Add(roKernel.Get<IBackfill>());
            processModes.Add(roKernel.Get<IConfigRestore>());
            processModes.Add(roKernel.Get<IConfigCreation>());
            processModes.Add(roKernel.Get<IApplicationDeployer>());
            processModes.Add(roKernel.Get<IDoesNothing>());

            var processMode = args.MapArgsToProcessMode();

            var ourWeaponOfChoice = processModes.Where(x => x.processMode == processMode).First();

            ourWeaponOfChoice.Process();

        }
    }
}

Now. We don't have any switch statements or if statements. We could do a linq assembly bind to avoid having to add each item manually, and we don't have to keep changing any other class than the program class. Am pretty sure CastleWindsor could do this.

To prove it works. Here is the DoesNothing class and I will provide the output from the log file.

namespace IRFullDeploy.Steps
{
    public class DoesNothing : IDoesNothing
    {
        private static Logger logger = LogManager.GetCurrentClassLogger();

        public void BadConfig()
        {
            logger.Warn($"{nameof(DoesNothing)} {processMode}");

            // Does other stuff 
            // ...
        }

        public ProcessMode processMode => ProcessMode.Unknown;

        public void Process()
        {
            BadConfig();

        }
    }
}

Output from the log file

2020-01-10 12:57:45.7634 WARN DoesNothing Unknown 

What is the significance of this post?

Well, what has been achieved, is a way to add any number of objects into a common interface and open the entire application up to flexibly deciding which one to implement without having to go into complicated branching.

Written with StackEdit.

Oldest comments (1)

Collapse
 
zakwillis profile image
zakwillis

Have reread this and in some ways - it is a souped up factory pattern without the branching. This is the thing with patterns - they all become much of a muchness.