Click here to Skip to main content
13,256,716 members (53,738 online)
Click here to Skip to main content
Add your own
alternative version

Tagged as

Stats

41.8K views
71 bookmarked
Posted 2 Nov 2017

C# BAD PRACTICES: Learn How to Make a Good Code by Bad Example – Part 5

, 16 Nov 2017
Rate this:
Please Sign up or sign in to vote.
What is really the Open Closed Principle about?

Introduction

My name is Radoslaw Sadowski and I'm a Microsoft Certified Senior Software Developer and currently a Squad Leader.

Since the beginning of my career, I have been working with Microsoft technologies.

This is the fifth article in "C# BAD PRACTICES: Learn how to make a good code by bad example" series.

You can find my 4 previous articles in this series here:

  1. C# BAD PRACTICES: Learn how to make a good code by bad example
  2. C# BAD PRACTICES: Learn how to make a good code by bad example – Part 2
  3. C# BAD PRACTICES: Learn how to make a good code by bad example – Part 3
  4. C# BAD PRACTICES: Learn how to make a good code by bad example – Part 4

 

This article is about Open Closed Principle (OCP) in SOLID principles and you don't have to read my previous articles to understand it, but I encourage you to do so. :)

My motivation of writing this article is the fact that there is a huge confusion around this principle and many different interpretations of it.

This principle was confusing to myself as well and that's why I went deep inside this topic and will present my finding and thoughts about it here.

In my opinion, it is besides the Liskov Substitution Principle, the most difficult one (to fully understand) from SOLID.

From my experience, I can say that it is confusing, even senior engineers and most developers know only a definition of it without a deep understanding of why and when it is useful.

This may lead to blindly applying this rule which can make the code base bizarre.

I also saw on many internet forums that many people are presenting concrete situations and asking "does it violate OCP?" etc. These questions are sometimes answered, but as:

  • there are many different opinions
  • there is no source of truth
  • and it DEPENDS

the OCP is still kind of a mystery in my opinion.

In this article, I will try to:

  • present how I understand OCP
  • present violation of OCP and solution for highlighted problems
  • present examples when I would extend my code while applying requirement changes
  • present examples when I would modify my code while applying requirement changes

As always, I will use a real world example, first show a bad code (in my opinion) and then present a better solution.

Please REMEMBER that giving example of code, I'm not presenting an end to end functionality implementation, I'm only presenting an approach.

To understand this article, you will need:

  • basic knowledge of C# language
  • basic understanding of Dependency Injection design pattern and IOC containers
  • basic knowledge of NuGet packages

Additional note: when I'm using IMO acronym in the article I mean "In My Opinion" (added after Alex (RSA) comment).

Definition and the History

The Open Closed Principle was introduced in book "Object Oriented Software Construction" by Bertrand Meyer.

It says:

"software entities (classes, modules, functions, etc.) should be open for extension, but closed for modification ".

The suggested way of solving this problem was the inheritance.

This means that you should extend your components by creating subclasses, rather than modifying the existing code.

 

Robert C. Martin introduced a new way of solving this problem - abstraction.

So if you will use programming to abstraction, your code will be more flexible for future changes as you can replace or add a new implementation without modifying consumer of the class as the consumer sees the abstraction (abstract class or interface) instead of concrete implementation.

One additional thing is that OCP adds an exception to the main rule which is bug fixing. So if you have a bug in your component, it is allowed to modify the code to fix this bug.

How Do I Understand OCP?

My understanding of this principle is that it is all about designing functions, classes and modules.

What does it mean?

Well, IMO as an experienced developer, while creating a new functionality you should try to predict the future.

You should identify places in your components which will likely need some modifications in the future (requirements changes) and these modifications will be problematic (re-designing a module, high chance to introduce bugs in existing functionality).

Once you identify these places, you should find a solution which will make your code flexible to these changes.

So...

IMO, it doesn't mean that:

"You should never modify your existing class",

but:

"You should design your modules in a way that you won't have to do it when you want to extend functionality".

However, we are designing our code to be OCP compliant to be able then to extend our code instead of modifying it. So it would not make much sense to modify the code which was designed to be extendable. That's why following OCP during design phase is strictly related to the way of applying requirements changes in the future, which should be ideally done by extension not by modification.

So IMO, you can say that violation of OCP will be implementation of not-extendable code as well as modification of existing code. I will explain later about when the second one is OK in my opinion.

There are many techniques you can use to write OCP compliant code like:

  • Abstraction
  • Dependency Injection
  • Polymorphism/Inheritance
  • Composition
  • Strategy pattern
  • Passing delegate to function
  • etc.

The complete solution usually consists of a combination of them. Which of them you should use, depends on the situation. In my example, I'll use abstraction and Dependency Injection.

How Do I Understand OCP?

Why not just write the code which works and modify it when needed without this flexibility?

Because, every modification in the existing code:

  • is very risky and ideally, you should avoid it
  • can potentially introduce a bug in functionality, which was not meant to be modified
  • may lead to modification of your unit/integration tests
  • may lead to break backward compatibility (i.e., modification of class existing in NuGet package, which is used as a base class in external system which is using this package)

If your code is closed for modification and open for extension, you don't need to modify the existing code when a new requirement comes in, you just add new components and start using them using configuration.

3 Levels

In the original definition of the principle, it is said that this rule should be respected on 3 levels:

  • function level
  • class level
  • module level

Whereas function and class levels are clear by their names, my understanding of the "module level" is that the "module" in this case is a set of classes and dependencies between them, which creates some functionality.

So it's something more abstract than for example assembly. I saw many different interpretations of the "module" level, so I want to say clearly that in my opinion it doesn't say that you shouldn't add a new code to your existing assembly.

I will show in my example violation of OCP on each of these 3 levels.

When the Code Becomes Closed

In my opinion, component becomes closed when it's first time deployed to production. I assume that before release, it is still in the development phase.

Predicting Future and YAGNI

Obviously, everything written above assumes that we are living in an ideal world and developers are clairvoyants who can predict everything which will happen in the future.

In a real world, we will never predict all requirement changes which can come from the business.

We shouldn't even try, because it can lead to overengineering which is bad as well. As a result, we can end up with the extremely complex system and we will never need to use its flexibility/complexity.

But if while developing some functionality, we see that the nature of the feature is that it will need some extension or changes and it's only a matter of time when requirement for that will come, we should design our code in a way that we will be able to achieve this extension by adding new components, not modifying the existing ones.

That is the most difficult part. We need to find a balance between Open Closed Principle and YAGNI (You aren't gonna need it) principle.

Let's Code

Bad Example

As the code example, I chose a module which validates password.

The initial requirement is that password:

  • has to be longer than 8 characters
  • can't be the same as the user name
  • can't be the same as any of the passwords set by user in the past

Below I present an implementation, which violates the OCP on all 3 levels:

public class PasswordChangeModel
{
    public string NewPassword { get; set; }
    public List<string> PasswordHistoryItems { get; set; }
    public string Username { get; set; }
}

public class PasswordValidator
{
    private const int _minLength = 8;

    bool IsValid(PasswordChangeModel passwordChangeModel)
    {
        if (passwordChangeModel.NewPassword.Length < _minLength ||
            passwordChangeModel.NewPassword == passwordChangeModel.Username ||
            (passwordChangeModel.PasswordHistoryItems != null &&
            passwordChangeModel.PasswordHistoryItems.Contains(passwordChangeModel.NewPassword)))
        {
            return false;
        }

        return true;
    }
}

public class PasswordManager
{
    public bool ChangePassword()
    {
        var passwordValidator = new PasswordValidator();
        var isPasswordValid = passwordValidator.IsValid();

        // the rest of the logic
    }
}

The above code presents 3 classes:

  • PasswordChangeModel – object that contains all password change details
  • PasswordValidator – our validator class
  • PasswordManager – a consumer of PasswordValidator class

As it is very likely that password validation rules will change over time to improve security, our module should be open for extension. As we will not want to break our existing functionality while extending it, it should also be closed for modification.

The above example clearly violates the OCP on all 3 levels:

  • function level – Any requirement of a new rule or change in existing one will need a modification of IsValid method. It is very easy to introduce some stupid mistake in this function as even with only 3 rules, the code is unreadable. You can imagine how messy it would be with 10 rules.
  • class level – if we would want to change minimum password length to 10, we need to do a modification on a class level and modify a private field like this:

private const int _minLength = 10;

What's the problem?
There is no problem, if there is only one consumer of the PasswordValidator class (or all consumers need to use a minimum length of 10) and there in no subclass of PasswordValidator class.
But..
If the PasswordValidator class is used by many consumers, you will change a behaviour of all of them. Maybe in 2 applications, it should still be 8 and in the third one, it should be 10.
If the PasswordValidator class is a base class of other classes, you will change their behaviour as well. This might not be an intended behaviour.

Moreover, if you will consider requirement “can't be the same as any of passwords set by user in the past” you can assume that it will likely change to:
"can't be the same as any of X last passwords set by user", where x can be anything :D

PasswordChangeModel class doesn't allow to implement this functionality, as we don't have information about the order of password changes.

  • module level – Assuming that these 3 classes create a module, if we will want our consumer to start using a new implementation of our password validator, we have to modify consumer, as PasswordValidator implementation is not hidden after the interface and we will not be able to just inject into it a different implementation.

As you can see, our code clearly violates the OCP.
If we would like to create a reusable library which will be validating passwords in many applications in our company or in many companies, and distribute it using NuGet, the above implementation will very likely create trouble in the future.

Why?

Because, you won't be able to say:

  • what are the consumers of the PasswordValidator
  • what classes inherit from the PasswordValidator (unless you mark it as sealed (MSDN), but it's a kind of limitation)

That's why we will have to keep backward compatibility to avoid changing a behaviour of applications which are using our library.

But also, if our password validation module is a simple library, we can have multiple consumers and subclasses of the PasswordValidator class whose behaviour we don't want to change when extending functionality.

Ok, so to prove a violation of OCP, let's try to handle some requirement change which will likely come to us.

Now, as an addition to the existing validation, we have to add a new rule:

  • password cannot contain the following words: "password", "qwerty", "abc123"

As we don't care about OCP, we are just adding another rule to our PasswordValidator:

public class PasswordValidator
{
    private const int _minLength = 8;
    static List<string> BlackList = new List<string> { "password", "qwerty", "abc123" };

    bool IsValid(PasswordChangeModel passwordChangeModel)
    {
        if (passwordChangeModel.NewPassword.Length < _minLength ||
            passwordChangeModel.NewPassword == passwordChangeModel.Username ||
            (passwordChangeModel.PasswordHistoryItems != null &&
            passwordChangeModel.PasswordHistoryItems.Contains(passwordChangeModel.NewPassword)) ||
            (BlackList != null && BlackList.Contains(passwordChangeModel.NewPassword)))
        {
            return false;
        }

        return true;
    }
}

Now, some unit tests of IsValid method which were containing newly introduced forbidden words (and were passing) will start failing.
But the bigger problem is that this change will affect ALL consumers and subclasses of the PasswordValidator. So if we introduced a bug, what is very easy in such fragile code, it can break the existing functionality.

Moreover, every time we will want to add/remove/change a word in blacklist, you will have to modify PasswordValidator class, which will affect all consumers and derived classes.

You can think of many different rules which we will need to implement after few requirement changes like:

  • max password length
  • must contain special character
  • can't have following digits
  • must include uppercase
  • must include lowercase
  • must include 2 digits
  • can't be the same as any of X last passwords set by user (mentioned above)
  • and many many more

Our code is not great to handle any extension or modification.
Why?
Because the developer just implemented directly what was said in the requirements without thinking too much. Keeping things too simple, he created not extendable code.

Let's try a better approach!

Better Approach

Let's start from the beginning with a better approach:

public class PasswordChangeModel
{
    public string NewPassword { get; set; }
    public List<PasswordHistoryItem> PasswordHistoryItems { get; set; }
    public string Username { get; set; }
}

public class PasswordHistoryItem
{
    public string PasswordText { get; set; }
    public DateTime CreationDate { get; set; }
}

public interface IPasswordValidator
{
    bool IsValid(PasswordChangeModel passwordChangeModel);
}

public interface IPasswordValidationRule
{
    bool IsValid(PasswordChangeModel passwordChangeModel);
}

public class PasswordMinLengthRule : IPasswordValidationRule
{
    private readonly int _minLength;

    public PasswordMinLengthRule(int minLength)
    {
        _minLength = minLength;
    }

    public bool IsValid(PasswordChangeModel passwordChangeModel)
    {
        return passwordChangeModel.NewPassword.Length >= _minLength;
    }
}

public class PasswordUsernameRule : IPasswordValidationRule
{
    public bool IsValid(PasswordChangeModel passwordChangeModel)
    {
        return passwordChangeModel.NewPassword != passwordChangeModel.Username;
    }
}

public class PasswordHistoryRule : IPasswordValidationRule
{
    public bool IsValid(PasswordChangeModel passwordChangeModel)
    {
        return passwordChangeModel.PasswordHistoryItems == null || 
            !passwordChangeModel.PasswordHistoryItems.Any
            (x => x.PasswordText == passwordChangeModel.NewPassword);
    }
}

public class PasswordValidator : IPasswordValidator
{
    private List<IPasswordValidationRule> _validationRules;

    public PasswordValidator(List<IPasswordValidationRule> validationRules)
    {
        _validationRules = validationRules;
    }

    public bool IsValid(PasswordChangeModel passwordChangeModel)
    {
        foreach (var validationRule in _validationRules)
        {
            if (!validationRule.IsValid(passwordChangeModel))
            {
                return false;
            }
        }
        return true;
    }
}

Ok, so we've implement PasswordChangeModel class in a slightly different way. Now we have more information about old passwords. Now we have a creation date of each password from the past in the PasswordHistoryItem object. In the very likely case of changing requirement from:

  • can't be the same as any of the passwords set by user in the past

to:

  • can't be the same as any of X last passwords set by user

We have all information in place and we only need to add a new rule.

Next thing is introduction of 2 interfaces:

  • IPasswordValidator
  • IPasswordValidationRule

The first one is an abstraction layer between consumer and PasswordValidation class. Thanks to introducing it, we removed OCP violation on a module level (and tight coupling at the same time).
I will show it in consumer code later on.

The second interface is an abstraction between PasswordValidator and PasswordValidationRules:

  • PasswordMinLengthRule
  • PasswordUsernameRule
  • PasswordHistoryRule

As you can see, each rule logic was moved to a separate class, and these classes are implementing IPasswordValidationRule interface.

And finally the “heart”, PasswordValidator class, which now implements IPasswordValidator interface.

public class PasswordValidator : IPasswordValidator
{
    private List<IPasswordValidationRule> _validationRules;

    public PasswordValidator(List<IPasswordValidationRule> validationRules)
    {
        _validationRules = validationRules;
    }

    public bool IsValid(PasswordChangeModel passwordChangeModel)
    {
        foreach (var validationRule in _validationRules)
        {
            if (!validationRule.IsValid(passwordChangeModel))
            {
                return false;
            }
        }
        return true;
    }
}

You can see that now we're injecting all needed rules to the PasswordValidator class. This should be done in each application(host) configuration. I'm talking here about applications, which will be using validation module. So each application or even each consumer in the same application, can have a different set of rules configured (List<IPasswordValidationRule>).

Let's assume now that we have 2 consumers of our password validation feature:

public class Consumer1
{
    private readonly IPasswordValidator _passwordValidator;

    public Consumer1(IPasswordValidator passwordValidator)
    {
        _passwordValidator = passwordValidator;
    }

    void SomeMethod(PasswordChangeModel passwordChangeModel)
    {
        // some code

        _passwordValidator.IsValid(passwordChangeModel);

        // some code
    }
}

public class Consumer2
{
    private readonly IPasswordValidator _passwordValidator;

    public Consumer2(IPasswordValidator passwordValidator)
    {
        _passwordValidator = passwordValidator;
    }

    void SomeMethod(PasswordChangeModel passwordChangeModel)
    {
        // some code

        _passwordValidator.IsValid(passwordChangeModel);

        // some code
    }
}

You can see that we're now injecting IPasswordValidator implementation using Dependency Injection principle.

Thanks to that, we can freely change used by consumer password validation functionality by injecting a different implementation than PasswordValidator class. Our module is open for extension and closed for modification in case of password validation.

Before I will explain configuration of the module, let's try to implement a new requirement (new password validation rule):

  • password cannot contain the following words: "password", "qwerty", "abc123"

again, but in a new code base.

What we really need to do is to add a new implementation of IPasswordValidationRule:

public class PasswordBlackListRule : IPasswordValidationRule
{
    private readonly List<string> _blackList;

    public PasswordBlackListRule(List<string> blackList)
    {
        _blackList = blackList;
    }
    public bool IsValid(PasswordChangeModel passwordChangeModel)
    {
        return _blackList == null || !_blackList.Contains(passwordChangeModel.NewPassword);
    }
}

You can see that PasswordBlackListRule implementation is also open for extension and closed for modification itself as we're injecting a Black List and if we will have to modify it, we will do it in configuration, not in the rule implementation.

Let's now set up everything together!

I would suggest to set up a configuration of password validation module, using one of existing IoC containers. Below is an example using Ninject (ninject module configuration):

public class Bindings : NinjectModule
{
    public override void Load()
    {
        Bind<IPasswordValidator>().ToConstant(new PasswordValidator
             (GetPasswordRulesForConsumer1())).WhenInjectedInto<Consumer1>();
        Bind<IPasswordValidator>().ToConstant(new PasswordValidator
             (GetPasswordRulesForConsumer2())).WhenInjectedInto<Consumer2>();
    }

    private List<IPasswordValidationRule> GetPasswordRulesForConsumer1()
    {
        return new List<IPasswordValidationRule>()
        {
            { new PasswordMinLengthRule(8) },
            { new PasswordUsernameRule() },
            { new PasswordHistoryRule() }
        };
    }

    private List<IPasswordValidationRule> GetPasswordRulesForConsumer2()
    {
        return new List<IPasswordValidationRule>()
        {
            { new PasswordMinLengthRule(8) },
            { new PasswordUsernameRule() },
            { new PasswordHistoryRule() },
            { new PasswordBlackListRule(new List<string>() { "password", "qwerty", "abc123" }) }
        };
    }

}

As you can see above, we can have a different configurations (a set of rules for each consumer) even in the same application, thanks to WhenInjectedInto method from Ninject API. For people who are not familiar with Ninject, here is a link to the documentation: Ninject documentation

The first consumer will use password validator with 3 rules:

  • PasswordMinLengthRule
  • PasswordUsernameRule
  • PasswordHistoryRule

whereas the second consumer will use 4 validation rules:

  • PasswordMinLengthRule
  • PasswordUsernameRule
  • PasswordHistoryRule
  • PasswordBlackListRule

You can obviously implement Dependency Injection manually in your application, it depends on your needs.

So when we will want to:

  • add a new rule to the validation process– we need to create a new implementation of IPasswordValidationRule and add it in configuration – existing components don't change
  • add an existing rule to the validation process – change in configuration – existing components don't change
  • replace existing rule in the validation process – change in configuration – existing components don't change
  • remove rule from the validation process – change in configuration – existing components don't change

Even such flexible code doesn't protect from every requirement, but going further with flexibility IN MY OPINION will be overengineering and will violate YAGNI. And here, I would say we achieved a balance.

SOLID Go Nicely Together

As you can notice, in the example, I'm using also:

  • Dependency Inversion - Dependency Injection implementation
  • Single Responsibility -
    PasswordValidator - validates password using rules (1 responsibility)
    Particular rule – is checking only one thing

from SOLID principles.

Only a combination of SOLID rules will allow you to write more understandable, flexible and maintainable software.

More Examples

If you want to see another example of applying OCP, please read my article, where I describe why switch-case statement breaks OCP and how to re-write it to OCP compliant code:

To Modify or To Extend

As a natural continuation of designing modules as OCP compliant is applying requirement changes, in this paragraph, I will show concrete examples when I would/wouldn't modify the original code while doing that.

When I Would Extend

Let's imagine that we have implemented password validation library mentioned in the previous paragraph (the better, flexible version). We decided to share it as a NuGet package.

Example 1

Let's assume that in version 1 of nuget package, we've implemented PasswordMinLengthRule rule as below:

public class PasswordMinLengthRule : IPasswordValidationRule
{
    private const int _minLength = 8;

    public bool IsValid(PasswordChangeModel passwordChangeModel)
    {
        return passwordChangeModel.NewPassword.Length >= _minLength;
    }
}

Then, we realized that different applications want to use a different minimum length. This version obviously doesn't give such flexibility.

We would want to replace this implementation with the more flexible option:

public class PasswordMinLengthRule : IPasswordValidationRule
{
    private readonly int _minLength;

    public PasswordMinLengthRule(int minLength)
    {
        _minLength = minLength;
    }

    public bool IsValid(Password password)
    {
        return password.NewPassword.Length >= _minLength;
    }
}

Now, each application can set its own password minimum length while creating the rule object.

But how to replace the code with a better version...

What are the options?

Option 1: We can modify the existing class code – which breaks OCP.

Option 2: We can add a new rule with more flexible implementation beside the original one – which follows OCP:

public class PasswordConfigurableMinLengthRule : IPasswordValidationRule
{
    private readonly int _minLength;

    public PasswordConfigurableMinLengthRule(int minLength)
    {
        _minLength = minLength;
    }

    public bool IsValid(PasswordChangeModel passwordChangeModel)
    {
        return passwordChangeModel.NewPassword.Length >= _minLength;
    }
}

I would choose in this case Option 2 and follow OCP.
Why?
Because as this code is distributed as a package, I'm not aware of all callers and subclasses of original version of PasswordMinLengthRule class. So following Option1 and modifying existing code in version 2 of the package, I will potentially break existing code and force consumers of the class to modify it – read “lack of backward compatibility”. They may for example need to upgrade NuGet package version, because it also includes a very important bug fix.

But I would do one additional thing here, I would mark the existing class - PasswordMinLengthRule – as obsolete:

[Obsolete("You should not use this class anymore, 
there is more flexible version of this class - PasswordConfigurableMinLengthRule")]
public class PasswordMinLengthRule : IPasswordValidationRule
{
    private const int _minLength = 8;

    public bool IsValid(PasswordChangeModel passwordChangeModel)
    {
        return passwordChangeModel.NewPassword.Length >= _minLength;
    }
}

Thank to that, consumers of our library will be notified that they should start moving to a new version, however they are not presented with a fait accompli.

If you're not familiar with Obsolete attribute, please read MSDN.

When I Would Modify

Example 2

Now please forget the original example and imagine that this is your first version of PasswordMinLengthRule:

public class PasswordMinLengthRule : IPasswordValidationRule
{
    private readonly int _minLength;

    public PasswordMinLengthRule(int minLength)
    {
        _minLength = minLength;
    }

    public bool IsValid(PasswordChangeModel passwordChangeModel)
    {
        return passwordChangeModel.NewPassword.Length <= _minLength;
    }
}

You have an obvious bug:

return password.NewPassword.Length <= _minLength;

comparison sign is written incorrectly:

<=

instead of:

>=

It doesn't matter if it's a shared nuget package with 100 consumers, it doesn't matter if there are many classes which are inheriting from PasswordMinLengthRule or if there is only one caller and none class which inherits from PasswordMinLengthRule, you have to fix the obvious bug as soon as possible. Consuming the above code doesn't make sense.

So I would just modify this class and correct the comparison sign. As OCP allows to modify the code when fixing bugs, this solution is still OCP compliant.

Example 3

Let's start from the beginning again. Now imagine that we have 3 callers of the PasswordMinLengthRule class, and this is the initial implementation:

public class PasswordMinLengthRule : IPasswordValidationRule
{
    private const int _minLength = 8;

    public bool IsValid(Password password)
    {
        return password.NewPassword.Length >= _minLength;
    }
}

It is a class, that is only used in one solution (it isn't NuGet package), subclasses of this class don't exist and you own 3 applications which are using it.

There is a new requirement which says: “All 3 callers should now use 10 as a minimum password length”.

What can you do if you want to follow OCP (and not modify the original PasswordMinLengthRule class):

Option 1: Create a new implementation of PasswordMinLengthRule:

public class PasswordMin10LengthRule : IPasswordValidationRule
{
    private const int _minLength = 10;

    public bool IsValid(PasswordChangeModel passwordChangeModel)
    {
        return passwordChangeModel.NewPassword.Length >= _minLength;
    }
}

or more configurable:

public class PasswordConfigurableMinLengthRule : IPasswordValidationRule
{
    private readonly int _minLength;

    public PasswordConfigurableMinLengthRule(int minLength)
    {
        _minLength = minLength;
    }

    public bool IsValid(PasswordChangeModel passwordChangeModel)
    {
        return passwordChangeModel.NewPassword.Length >= _minLength;
    }
}

and start using this implementation in three callers.

Assuming that we are using Dependency Injection and we are injecting implementation of IPasswordValidationRule, we only have to replace an implementation in IOC container/manual configuration.

Note that if we will do that, old PasswordMinLengthRule will become an orphan class which you have to maintain, and everybody who will read the code will be wondering why this class exists if it's not used.

What is the difference between adding a new class and stopping using the old one and modifying the old one? There is no difference. :)

I would not follow any of these two options, neither OCP principle in this case. I would use the code from option 2 and just replace the original code with the code below:

public class PasswordMinLengthRule : IPasswordValidationRule
{
    private readonly int _minLength;

    public PasswordMinLengthRule(int minLength)
    {
        _minLength = minLength;
    }

    public bool IsValid(PasswordChangeModel passwordChangeModel)
    {
        return passwordChangeModel.NewPassword.Length >= _minLength;
    }
}

Why?

Because I don't see any benefit of keeping the first implementation if after change, it won't be used. Moreover, we obviously don't want anybody to use this inflexible version of the code by mistake in the future as the new version can do everything that the old version does, but in a more flexible way.

So to sum up, I will do a refactoring of the code (breaking OCP) and start using the new version.

Conclusion

To sum up, I presented MY opinion on a very broad topic, which is Open Closed Principle. You have to remember, that it's only one of the guidelines to achieve more understandable, flexible, maintainable and less “buggy” software, it IS NOT the rule you have to always blindly apply.

It makes some time to get the experience, which allows to design properly your components. I hope that my examples illustrated the idea of OCP clearly and will be helpful to judge what kind of approach to take in similar situations. However, you have to always use common sense when you make a decision what solution will be the best in the particular case and find a balance between OCP and YAGNI.

When making a decision to extend or to modify the existing class, always consider all its potential consumers and subclasses, this will allow you to take the best approach for the particular case.

Thanks!

If you have any questions related to the article, don't hesitate to contact me!

Sources

Used in the article information:

Source of images used in the article:

License

This article, along with any associated source code and files, is licensed under The Code Project Open License (CPOL)

Share

About the Author

Passionate, Microsoft Certified Software Developer(Senior),
having Masters Degree in Information Technology.

You may also be interested in...

Pro
Pro

Comments and Discussions

 
Praisenice article Pin
karthikvaduganathan14-Nov-17 16:37
memberkarthikvaduganathan14-Nov-17 16:37 
GeneralRe: nice article Pin
Radosław Sadowski14-Nov-17 22:54
memberRadosław Sadowski14-Nov-17 22:54 
GeneralNice article Pin
charithrasanga10-Nov-17 18:35
membercharithrasanga10-Nov-17 18:35 
GeneralRe: Nice article Pin
Radosław Sadowski11-Nov-17 0:14
memberRadosław Sadowski11-Nov-17 0:14 
QuestionPart 6? Pin
Consulting Mechanic10-Nov-17 11:02
memberConsulting Mechanic10-Nov-17 11:02 
AnswerRe: Part 6? Pin
Radosław Sadowski10-Nov-17 11:51
memberRadosław Sadowski10-Nov-17 11:51 
QuestionTypos, comments Pin
Member 123234789-Nov-17 1:36
memberMember 123234789-Nov-17 1:36 
AnswerRe: Typos, comments Pin
Radosław Sadowski9-Nov-17 2:06
memberRadosław Sadowski9-Nov-17 2:06 
QuestionRefactoring work around Pin
Rian Meier7-Nov-17 20:09
professionalRian Meier7-Nov-17 20:09 
AnswerRe: Refactoring work around Pin
Radosław Sadowski8-Nov-17 7:35
memberRadosław Sadowski8-Nov-17 7:35 

General General    News News    Suggestion Suggestion    Question Question    Bug Bug    Answer Answer    Joke Joke    Praise Praise    Rant Rant    Admin Admin   

Use Ctrl+Left/Right to switch messages, Ctrl+Up/Down to switch threads, Ctrl+Shift+Left/Right to switch pages.

Permalink | Advertise | Privacy | Terms of Use | Mobile
Web01 | 2.8.171114.1 | Last Updated 16 Nov 2017
Article Copyright 2017 by Radosław Sadowski
Everything else Copyright © CodeProject, 1999-2017
Layout: fixed | fluid