Question about repository pattern

Question

Is this a good or bad idea?

public interface IRepository<T>
{
    ...
    IList<T> Get(Func<T, bool> predicate)
    ...
}

I mean - it seems really powerful addition and i could implement it too (at least - to some level) but my guts tells me it's kind a wrong. Could anyone enlighten me?

Answer 1

I think this is better :

public interface IRepository<T>
{
IQueryable<T> GetAll();
}

then you can write your custom queries as :

var employees=Repositroy<Employee>.GetAll().Where(emp=>emp.Salary>10000).Select(emp=>emp).ToList();

Answer 2

Well, it certainly is a powerful feature. The problem is: introducing it in the interface will force all implementations of IRepository to provide a suitable definition -- which might not be possible (or at least really hard) depending on the implementation (say, an implementation backed by a database connection or something).

Maybe, you should instead do

public interface IRepository<T>
{
    ...
}

public interface IAdvancedRepository<T>: IRepository<T> 
{
    IList<T> Get(Func<T, bool> predicate)
}

and provide Get only, if the implementation can do it efficiently.

Answer 3

I tend to link 'Get' with getting one simple instance, not a list. Therefore, when I have a method which returns a collection, I usually name that method 'Find' or something like that.

(Note that FxCop will tell you it is a bad idea to use 'Get' as a function name).

Ah, Ok, I missed the point. :) I would not create a method which takes a a criteria as an argument. I'd rather have specialized methods on my repository instead of passing in all sorts of criteria. I know, it is not as flexible, but it is just more explicit.

Answer 4

For me it smells like you don't really need a repository as .NET devs usually understand it.

I think you need something like this:

public interface IStore {
    // Low-level abstraction over a real database
}

public interface IAction<TInput, TResult> { // Or maybe ICommand?
    TResult Execute(TInput input);
}

// then:
public interface IGetCustomerById : IAction<int, Customer> {
}
public class GetCustomerById : IGetCustomerById {
    IStore store;
    public GetCustomerById(IStore store) {
        this.store = store;
    }

    public Customer Execute(int id) {
        return store.ResultOfExecutedSqlOrWhatever(id);
    }
}
// it might not only be READ action but more complex one, reusing other actions too
public class ApproveCustomer<ApprovalInfo,ApprovalResult> : IAction {
    IStore store;
    IGetCustomerById getCustomer;
    public ApproveCustomer(IStore store, IGetCustomerById getCustomer) {
        this.store = store;
        this.getCustomer = getCustomer;
    }

    public ApprovalResult Execute(ApprovalInfo approval) {
        var customer = getCustomer.Execute(approval.CustomerId);
        var result = new ApprovalResult();
        if (customer == null) {
            result.Message = "No such customer";
            return result;
        }
        if (!customer.IsEligibleForSomething()) { // This can be another action!
            result.Message = "Not eligible"
            return result;
        }
        result.Message = "Successfully approved"
        result.IsApproved = true;
        return result;
    }
}

Additional benefit here is that all these actions can be easily used with dependency injection.