Making a class follow OCP - Factoring functions into objects

Question

I have a class to which I'm constantly adding to.

public class OrderRepository{
    public void Add(IEnumerable<Order> orders){}
    public void Update(IEnumerable<Order> orders){}
    public void Remove(IEnumerable<Order> orders){}
    public void Expedite(IEnumerable<Order> orders){}
    public void GetOrderData(Order order, DateTime start, DateTime end)
    etc...
}

It occurred to me that this class is not Open-Closed, because of all of these new features being added. So I thought about closing this class against this change by encapsulating these functions into Request objects. I end up with something like:

public abstract class RequestBase{}

public class AddRequest : RequestBase{}

etc...

public class OrderRepository{
    public void ProcessRequest(RequestBase request){}
}

Which makes OrderRepository open for extension and closed for modification. However, I quickly ran into a few issues with this:

1.) The data the that request needs to operate on is both user-supplied (run-time) and dependency injection-supplied. I obviously can't satisfy both with one constructor. I can't do:

public class AddRequest{
    public AddRequest(IEnumerable<Order> orders, int UserSuppliedContextArg1, DependencyInjectionArg1, DependencyInjectionArg2);
}

and call that. I would want a way for the DI framework to "partially" construct an object for me, and let me do the rest. I don't see any way of doing that however. I saw a blog which called this concept "variable constructor injection."

2.) The next thing I thought about was to split this into 2 separate classes. The user would create and fill a RequestContext, and then pass that into the Repository, which would create a RequestProcessor (can't think of a better name) off of it. I thought about doing:

public abstract class RequestContextBase<T> where T : RequestProcessorBase{}

public class AddRequestContext : RequestContextBase<AddRequestProcessor>

public class OrderRepository{
    public void ProcessRequest<T>(RequestBase<T> request){
     var requestProcessor = IoC.Create<T>();
    }
}

and that was a good first step. However, the request processor needs the exact type of context it's storing, which I don't have here. I could use a dictionary of types to types, but that defeats the purpose of being Open-Closed .So I end up having to do something like:

public class RequestProcessorBase<TRequestContext, TRequestProcessorBase> where TRequestContext : RequestContextBase<TRequestProcessorBase>

This is weird and I'm usually not fond of the curiously recurring template pattern. In addition, the idea of the user filling up a context and asking me to make a request of it seems strange, although that may be just a naming issue.

3.) I thought about getting rid of all of the above and just having:

public AddRequest{
    public AddRequest(DependencyInjectionArg1, DependencyInjectionArg2, ...){}

    public void PackArgs(UserSuppliedContextArg1, UserSuppliedContextArg2, UserSuppliedContextArg3, ...){}
}

which is not bad, but the API is ugly. Now the clients of this object need to "construct" it twice, as it were. And if they forget to call PackArgs, I have to throw an exception of some sort.

I could go on, but these are the most confusing issues I have at the moment. Any ideas?

Answer 1

Ayende has a few posts on this subject.

Basically what you want to do is detach your query from your repository, and turn your query into something you compose. With a query constructed by composition, you can extend it with ease to add new ways to query without having to add new methods to your Repository. You can end up with something like this:

public class Repository<T>
{
   T Find(IQueryCriteria queryCriteria);
}

I haven't actually done this in NHibernate yet, but we did this with LLBLGenPro and it worked out really well. We used a fluent interface for our query objects so we could write query criteria like this:

var query = new EmployeeQuery()
   .WithLastName("Holmes")
   .And()
   .InDepartment("Information Systems");

var employee = repository.Find(query);

Extending the capabilities of the repository then amounted to simply adding new methods on the query object.

Answer 2

A repository is part of your domain. Making it generic, while tempting, defeats its purpose as a home to operations in the ubiquitous language. If you can do anything with a repository, you've obfuscated its intent.

If a class follows SRP, "constantly adding to" it violates that by definition. This indicates the operations you are introducing may be better addressed by services or should otherwise be decoupled from the repository.

Edit in response to comment

You want to keep your ubiquitous language out in the open, while ensuring classes have the minimum amount of responsibility.

Breaking out operations from the repository would be the first step. You could do something like this:

public interface IOrderExpeditionService
{
    void Expedite(IEnumerable<Order> orders);
}

public interface IOrderDataService
{
    void GetOrderData(Order order, DateTime start, DateTime end);
}