Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

How to not violate Open-Close Principle when new feature is requested?

I need to add a new functionality in a project and I am trying to do this in the best way.

So new feature = > Open close principle. I am supposed not to change existing code, am I right ?

Here is the interface:

public interface IEcuStatisticsExportManager
{
    void ExportStatistics(string datasource, bool modifiedOnly, string userName);
}

There is a class already implementing this interface:

public class EcuStatisticsExportManager : IEcuStatisticsExportManager
{
    public void ExportStatistics(string datasource, bool includeAll, string userName)
    {
        //Actual behavior of this method allow us to export statistics for 
        //only one data source. We need to extend this by allowing the user 
        //to export statistics for multiple data sources.

        //Another new feature will be an option to export statistics for
        //all data sources we have in the database
    }
}

Having those things in mind, the interface will have to look like this afterwards:

public interface IEcuStatisticsExportManager
{
    void ExportStatistics(string datasource[], bool exportAll, bool modifiedOnly, string userName);
}

I am struggling my brain with the following things:

How can I respect Open Close Principle as I can't override the method where I need the new behavior ?

How I will respect the principle as I will have to make changes on the interface ?

Please help me to do this in the best way.

Best regards.

like image 832
Adrian Chiritescu Avatar asked Mar 07 '23 17:03

Adrian Chiritescu


2 Answers

Open Closed Principle is something that should be thought of when you design the architecture of the project, and at least from my humble experience it would never cover 100% of the cases. When you need to add a feature, there is always a chance that the design does not support this feature and therefore the principle must be broken. Usually about 30% of the work is by the design, the rest of the work is hacking that design. In general, I would say that this principle should be kept only when its possible and reasonable to keep it. The main problem with concepts like SOLID and design patterns is that people are always looking for rules of thumb, and without understanding why you are working according to a certain rule, following it can do more harm than good.

Tmho, what you should ask yourself is if it makes sense to keep that rule in this given case in your given system in your given business case. In this case it could make sense to add the void ExportStatistics(string datasource[], bool exportAll, bool modifiedOnly, string userName); to the existing object in addition to your old method and use overloading, thus the new method will use the old one without modifying it, if its possible, probably by getting all datasources from the DB if needed, running a foreach loop on the datasources and call the old method on each datasource, and applying the necessary changes to the data afterwards based on the modifiedOnly parameter. That would save you from a lot of potential bugs and testing, since the actual tested export method was not touched. On the other hand, in many cases it will result in reduced performance, or it might prevent you from making the process transactional. In that case, is perfomance important here? Do all your export operations must be transactional? What amount of code would have to be added and maintained if you do it one way compared to the other, and do you have the manpower to maintain it? Is it more important than the time it will save you in bugs and tests? You can adjust these questions to your case. Only you know the answers to that, and only based on these answers you should decide. SOLID is not the bible (and therefore should not be ignored completely), it should be used if and only if its efficient to your case.

As @rory.ap mentioned, Its also very important not to change an interface that different objects are implementing in different solutions, since that would would be a breaking change - these solutions would not build. If that is the case you should either:

  • Create a new interface, as @roy.ap suggested
  • Add the method you need to the object without an interface
  • Coordinate the breaking change with all of the teams in your organizations in order make sure all other projects are updated accordingly. Then check your build server to see that all build scenarios can build successfully
Again, the right option in this case is entirely dependent on the specific situation and organization you are in.
like image 172
Yuval Perelman Avatar answered Mar 11 '23 23:03

Yuval Perelman


Your proposed changes will run afoul of the open/closed principal. Open for extension, closed for modification. You are modifying the signature of the method in the interface which is a breaking change. None of the existing projects which implement that interface will work after you deploy that change.

Depending on your needs, you could create a second interface, e.g. IEcuStatisticsExportManagerExtended and put your new method in there. Then classes could implement one or both of your interfaces as needed.

like image 24
rory.ap Avatar answered Mar 11 '23 23:03

rory.ap