4

I understand OCP in SOLID can be applied when we have same set of interfaces defined via abstract type, so that we can have varying implementations of those interfaces via concrete types.

For example, I have my business logic working with IRepository with defined set of operations:

interface IRepository<T>{
void save(T item);
T Get(int Id);}

Its possible to extend my application to support new type of repository - say from SQLRepository to OracleRepository without any modification to my code business logic.

However, If I need to provide Delete functionality - am not sure what design can help to adhere to OCP.

  • Are all current implementations of IRepository applicable to a Delete function? If so, and if you need Delete functionality, you simply must alter IRepository. Otherwise, you should make a clear logical division between when you need Delete and when you don't by introducing a new interface extending from IRepository which allows you to delete. Throughout your program, request the least specific interface whenever possible. – Neil Mar 6 '19 at 14:25
6

You could have another interface that supplies delete functionality. After all, not everything is going to need the delete functionality. This plays nice with the Interface Segregation Principle too. A new interface can be the combination of the two, which users could request if they need all of that functionality. If your language has traits/mixins, you could then write the delete functionality in isolation and mix it in.

But in most cases that is dumb. You'd be making abstractions that provide no benefit. You're solving problems that aren't problems yet and probably won't ever become problems. You'd be making your code harder to reason about and harder to maintain. In most cases, you should just add the delete method, OCP be damned.

The Open-Closed Principle and its kin are guidelines. They're there to help guide people to a better solution. But the better solution is the goal, not adherence to some rule.

  • 4
    The OCP is also less about “never change your code” and more about “consider designing your software in a way so that you can safely extend it without breaking backwards compatibility”. To which an application developer says: “what backwards compatibility? I don't need any compatibility because I control all dependent code and can refactor whatever I need. YAGNI.” – amon Mar 6 '19 at 19:20
  • @amon - still, it's good to make that refactor small/easy. Otherwise it doesn't get done or causes bugs. – Telastyn Mar 6 '19 at 19:31
3

Requirements should never know that types exist at all.

Your customer should never come to you asking for you to do something to a type because they should have never seen a type.

That said you can add a completely different type to do your new thing. You can create something new that does both the old and new thing. You don't have to break the old thing to add the new thing.

That's what the open closed principle is trying to say. The hardest part is just coming up with good names for the new things.

  • 2
    I think you are taking the term “requirement” a bit too literally? – amon Mar 6 '19 at 19:16
  • @amon I'm trying to drive home the point that the problem here is in the mind of the coder and no where else. When you think this way you've started letting your code dictate to you what you can and can't do. No, your codebase doesn't get to set the goal. It only decides how much pain is involved in getting there. – candied_orange Mar 6 '19 at 19:49
  • 1
    Did OP mention user requirement ? What about interoperability requirements, or technical requirements ? – Christophe Mar 6 '19 at 19:51
  • @Christophe I address that in the second half. If you're adding something new nothing knows it exists so you can add it wherever you like. – candied_orange Mar 6 '19 at 19:53
1

The simplest way to extend that would be to have something like:

interface IRepository<T>
{
    void save(T item);
    T Get(int Id);
    void delete(int id);
}

or:

interface IRepository<T>
{
    void save(T item);
    T Get(int Id);
}
interface IRepository2<T>
    : IRepository<T>
{
    void delete(int id);
}

The first example is perhaps the most direct approach. Simply manadate that all providers must support this behaviour. It presumes:

  • you have full control of your code base,
  • it makes sense for all of your repository providers to provide a delete(). There may be providers where this is less sensible, such as an append-only log.
  • you have the time and resources to update the providers.
  • and determine what should be done with providers that no longer meet the requirements of the interface. This could trigger potentially massive rewrites.

The second example is much more humble. For the most part it leaves your current code base alone, and allows providers to provide a larger wealth of services while still being backwards compatible. It comes at the cost of increasing the complexity of the code base. It is for situations where either:

  • your control is not so great over your code base,
  • there are providers where delete() does not make sense,
  • or you do not have the resources currently to refactor your system.

The naming in the second example though is abysmal - I just do not know enough about your system. If it is at all reasonable or possible, I would suggest renaming IRepository and IRepository2 to something more reflective of their roles, say:

  • IRepository -> IAppendRepository
  • IRepository2 -> IRepository

This should only be a minor refactor, although it can have wide-reaching implications. Particularly if this is in a library referenced accross many applications. Perhaps you should perform a stepping stone release that prevents name collisions, while still improve the clarity of the names:

  • IRepository -> IAppendRepository
  • IRepository2 -> IRepositoryWithDelete
1

API and framework developers, whom invest all their efforts in building infrastructure and core code everyone else will rely on, must be extra careful on not breaking backwards compatilbity. It's their job to make sure they honor their design.

Adding new operations, even if they don't break logic, might still break API consumers. Example, plug-ins, reflection, dependency injection, and many other things might detect the new changed interface is no longer the same as the loaded one, thus throwing security or load exceptions.

When this extra care is needed, you would usually create a new interface with the new methods, then make your old interfaces to extend the new ones. On documentation, you can warn users that pre-existent interfaces were "retrofit" with new stuff. That should give heads up, and it's usually gentler with things like DI, reflection, etc.

However, when you are the owner of the entire code, including the consumers of your interfaces, it's a no-brain to simply append the methods in the original interface and update all dependant code. It's easier and it should not be considered "less professional" (as long as it preserves the interface segregation principle)

1

This is not about Open-Close Principle(OCP), this is all about Interface Segregation Principle(ISP) in SOLID.

ISP stands for your new function should not effect your existing code which does not need this function. Simply, there is A class code and do not disturb it by adding new function if it doesn't need.

Assume that there are Log and Customer class. Log class does not need to Delete function but Customer does. In this case, if you add Delete function to IRepository<T>, you violate ISP because Log class does not need this function and you force to add this redundant function for it.

If you need IRepository<T> having all functions, then reconsider Log class should implement this interface or another(s).

OCP stands for creating your code without needing(close) to change your code but it is open to add new functionality.

Check out below SomeEnum and SomeClass:

public enum SomeEnum
{
    Type1,
    Type2
}

public class SomeClass
{
    public void SomeFunction(SomeEnum someEnum)
    {
        if(someEnum == SomeEnum.Type1)
        {
            // do something
        }
        else
        {
            // do another thing.
        }
    }
}

Even use else statements for the rest of thing, in this case else statement in SomeFunction represents SomeEnum.Type2. If you write your code like that, it is not close because adding Type3 to SomeEnum will effect this function. To avoid this problem, we can change if-else statement or use switch-case statements. It is also open to add new things.

public class SomeClass
{
    public void SomeFunction(SomeEnum someEnum)
    {

        switch(someEnum)
        {
            case SomeEnum.Type1:
                // do something for Type1
                break;
            case SomeEnum.Type2:
                // do something for Type2
                break;
            // case SomeEnum.Type3: 
            // open to adding type 3. Even don't add, there is no error or 
            // thing violates business rule.
                // do something for Type3
            //    break;     
        }
    }
}

Conclusion, you don't violate OCP, it is actually ISP and if your all classes which implement IRepository<T> should have Delete function, then implement it to IRepository<T>, otherwise create new interface that has Delete function and implement IRepository<T> and use it.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.