Does this break the Open/Closed Principle?

Question

I have spent most of my afternoon reading on the Open/Closed Principle, and I can't seem to understand it fully. Here are some referred articles I have read already and it seems as I missed something.

1. Understanding the Open Closed Principle
2. The end of dependency injection - who creates the dependencies?

Let's say I have a base generic repository which exposes some of the most generic methods expected to fit any needs from a repository.

Repository

public abstract class Repository<TModel> where TModel : class {
    protected Repository() { }

    public abstract IList<TModel> FilterBy(
        Expression<Func<TModel, bool>> filterExpression);
    public abstract IList<TModel> GetAll();
    public abstract TModel GetById(int id);
    public abstract Save(TModel instance);
}

Then, I wish to specialize into a ProductRepository.

ProductRepository

public abstract class ProductRepository : Repository<Product> {
    protected ProductRepository() : base() { }
}

Let's assume that I have all I need from the base repository here. If so, then I feel like I'm not breaking the Open/Closed Principle, because I'm not defining any new members or the like.

However, should I need a special kind of repository, let's say an AlertLevelConfigurationRepository, and the business requirement states that I can have only one AlertLevelConfiguration at a time. So, the repository needs to always get the current configuration.

AlertLevelConfigurationRepository

public abstract class AlertLevelConfigurationRepository 
    : Repository<AlertLevelConfiguration> {
    protected AlertLevelConfigurationRepository() : base() { }

    public abstract AlertLevelConfiguration GetCurrent();
} 

Now, I feel like I break the Open/Closed Principle because of this new method because this class is a modified derived type from its ancestor. It is modified as the base definition of a repository doesn't offer this GetCurrent method. Besides, I'm quite sure I will never use the any of the base method, given the exception of the Save method, since the alter level configuration can be, well, configurable!

In the end, I wonder whether I understand the Open/Closed Principle, and somehow I doubt I do.

I'd like to know if this is an example where the principle is broken, and if not, then I'd like to get the some explanations over the principle itself.

Answer 1

What you have there looks like the definition of "Open/Closed Principle" - the Repository class is Open to extension, but Closed to modification. You can add new functionality by extending it (in the form of new subclasses) without having to modify the Repository class. Adding the GetCurrent() call to the Alert subclass is part of the "Open to extension" part of the principle.

Open/Closed is about the specific class being Open/Closed, not the entire inheritance hierarchy. You want to write the class once, and only change it for one reason (Single Responsibility Principle).

You raise a separate issue:

"Besides, I'm quite sure I will never use the any of the base method, given the exception of the Save method, since the alter level configuration can be, well, configurable!"

This is a sign of a poorly designed inheritance hierarchy, or choosing to inherit from the incorrect class. If you're inheriting a lot of functionality you don't need or don't want to use, it's not the right base class, or that class is doing too much (Single Responsibility Principle violation).

What you probably want is to encapsulate some of this functionality - like the GetAll() and GetById() calls that your Alert class doesn't need - into separate classes that only the derived classes that need that functionality will take in as dependencies (assuming you're using DI). Or possibly put that functionality into a class that derives from Repository and have classes that need it derive from that class (I'd prefer the Composition solution over the inheritance solution though).