Open / Closed Principle - How to deal with this Switch?

Question

I have been looking into the open closed principle and it sounds good and so I wanted to exercise it's teachings. I looked at applying my new found knowledge to an existing project and have become a little stuck right away.

If a new UserType comes along (And this is Very likely), this will need to be changed, it is not yet closed to modification. How could one get round this?

From what I have read, it sounds like I should be implementing a factory here instead of applying the OCP?

Factory which breaks Open-closed principle

 private void BuildUserTree(User user)
    {
        switch (user.UserType)
        {
            case UserType.FreeLoader:
                BuildFreeLoaderTree();
                break;
            case UserType.Premium:
                BuildPremiumTree();
                break;
            case UserType.Unlimited:
                BuildUnlimitedTree();
                break;
            default:
                throw new Exception("No UserType set");

        }
    }

Thanks, Kohan

Eozoic

Answer 1

Like any 'principle' OCP is not a rule that you must obey on all occasions.

We're told to 'Favour composition over inheritance', and yet patterns like decorator and composite openly promote inheritance.

Similarly, we are told to 'Program to an interface, not an implemenation, and yet, at some point in our application we're going to have to instantiate a concrete object of some description.

Your solution is a classic factory idiom (if not quite the full factory method or abstract factory pattern). This is what it is intended to do. Trying to apply OCP to it doesn't make sense.

In fact, by creating this method, you may actually facilitate OCP in some other part of your code-base. Some other class or classes in your app could now obey OCP now you have separated the creation.

Changsha

Fair point, I thought as much. It's just when you learn something new you go out there and try and use the knowledge right away to back it up in your mind. I'll keep looking for a more suitable section of code to re-factor. Cheers. – Eozoic

No worries. Good thought exercise for me too. I'm in the process of learning patterns too :-) – Changsha

While it's true that those aren't and shouldn't be strict rules, switches are often bad smell and other designs/patterns could be favored which are far more respectful to OCP without negative drawbacks whatsoever. Similarly and bc you mentioned it, I personally try to use interfaces everywhere I can (safe for instantiation which can even sometimes be done through depedence injection and alike). – Pokeberry

This is a great take. Sometimes we shouldn't follow the principles so we can follow the principles elsewhere. – Marquise

Answer 2

I would do as below:

abstract class User {
   .
   .
   .
   abstract public void buildTree
}

class FreeLoaderUser: User {
   override public void buildTree()
   {
   }
}

class PremiumUser: User {
   override public void buildTree()
   {
   }
}

 class UnlimitedUser: User {
   override public void buildTree()
   {
   }
}

then instead of a method and a switch case that needs to be modified every time you add a new user type and simply call:

user.buildTree();

then this way whenever you need to add a new user type you extend your code instead of modifying. you just add a new class for the new user type with no touch to previous classes.

thats what they call open closed and when you can manage to handle it why should you violate it ?

Humming

Letting the children implement their needs themselves is a key way to implement OCP. – Pokeberry

I think the main problem is when the children just adding new content. Like you Creating document, you need to add field title, author, date etc.. So at the end you have an article. – Pointtopoint

Answer 3

internal class UserTreeBuilder
{
    [ImportMany(AllowRecomposition=true)]
    public IEnumerable<IBuilder> Builders{ get; set; }

    public UserTreeBuilder()
    {
        // Load all builders from a MEF CompositionContainer
    }

    public void BuildUserTree(User user)
    {
        var builder = Builders.FirstOrDefault(b => b.CanHandleUserType(user.UserType));
    
        if(builder == null)
        {
            throw new Exception("No UserType set");
        }else{
            builder.BuildTree();
        }
    }
}

The list of available builders could be build using MEF MEF, migrated from Codeplex

Argile

+1 for dynamic factory. See also. – Austen

Answer 4

To eliminate the type switch you have to move responsibilities back to the type that requires a type specific action. This type, in your case the "User", has all the information regarding himself and can easily invoke the proper operation based on this knowledge. And you have to make use of inheritance.

In your case you would have to reflect the user types via simple inheritance or via composition. Your "User" would than have a property "UserType", like in your example, but instead of making it just an "Enum" like type, it becomes a complex type that inherits an "IUserType" interface and knows how to construct its specific dependencies ("UserType" implements "IUserType"). The "IUserType" could expose type specific attributes via a property (e.g. "IUserType.TypeSpecificTree" that returns an "ITypeSpecificTree").

So when in your example a "User" is promoted to premium, you would just set the property to a new instance of the concrete "IUserType" implementation (e.g. PremiumUserType") that executes its specific actions like building the premium tree (an "ITypeSpecificTree" implementation) from your example as well as constructing associated types.

This way the switch statement is eliminated by using composition and inheritance. We transformed the complex "UserType" property into a separate class and then moved type specific responsibilities to the type itself. The inheritance and especially dependency inversion helped to operate on an object (e.g. getting the user type specific information like (User.IUserType.IUserSpecificTree") without knowing the concrete type. This helped to ensure that we are open for extension. Inheritance also helped to encapsulate type specific behavior to make our code close for modification.

If we need to make changes to how the type specific tree is generated or how this user type behaves, we would only touch the associated "IUserType" implementation but never the "User". If new user types are added (extension), they will have to implement the base interface "IUserType" and no other code, like switch-statements, must be touched to make it work and no more type checks are required. And to make it complete and to offer some more extensibility, the "User" class should also implement an interface e.g. "IUser" that exposes the user type (e.g. "IUser.IUserType").

Hillis