Choose the right type for method GetById

I designed my repository class and I need some advice from you. Now I have a method GetByID that returns a generic type:

public T GetById(object id)
 {
    return this.Entities.Find(id);
 }

I'm going to use OData to filter data, and for that (when I'm using $expand for single row) I need to do something like this:

SingleResult.Create(repository.GetByID(id));

But of course I received an error because SingleResult needs a type IQueryable.

So, I decided to change my GetByID to:

public IQueryable GetById(object id)
{
    return this.Entities.FirstOrDefault(p => p.ID == (int)id);
}

Could you please tell me if is it the right way? What type should be returned from GetByID?

I prefer to use T GetById because it is more correct in my opinion, but I don't know.

If you're only going to make the minimal required change

Accommodate the IQueryable requirement in the layer where it is needed, not in your repository layer. That will probably be some object that takes in an entity, wraps it in an IEnumerable, and calls AsQueryable() on it. (Disclaimer: This may or may not work. I haven't tested it.) If you were to do otherwise and change your repo signature to use IQueryable, that would be coupling the repository layer to downstream requirements, which you are correct in wanting to avoid.

A side pet peeve

Please avoid using object in signatures if you're going to use generics in the same class and you don't have a reason to box (I believe DbSet has to use reflection to find the key so it has a boxing requirement, but your code does not require boxing). Use strong typing all the way until you absolutely must cast to object. You'll thank me later. And make the parameter names expressive:

public TEntity GetById(TKey id)
{
    return this.Entities.Find((object)id);
}

Digging deeper

It seems a bit wonky and counterintuitive to have to pass an IQueryable to SingleResult. But come! Join me on a journey in pursuit of greater understanding of this "IQueryable".

An IQueryable is a bit more than just a collection. It functions as a sort of gateway to the data you're after, letting you query that data without having to use its unique language (SQL, or OData queries, what have you). This is accomplished through an associated query provider, which translates the lambda expressions you provide it to the language of your data store.

More importantly, that query provider decides how to compose and manage queries, and that includes utilizing deferred evaluation. The fact that you can pass it a query from your EF context is its way of telling you "I got this bro. This IQueryable has all I need to pull the data from here when I need it." It will then pull the data it needs for the OData query it's building on demand.

If you instead return it an entity, you have eagerly loaded that entity from the DB yourself, when you may not have needed to. Or at the very least, before you needed to.

A Catch-22

Using the generic repository here directly pits the benefits of IQueryable (deferred evaluation, the need for less "intermediary" code) against the reusability of a library that abstracts data access away from queries (is this even being reused?). In order for these two to coexist, some compromises are going to have to be made, compromises that may result in an extremely mediocre "solution."

A simpler appeal

If you look at the way the OData provider functionality is designed, it's evident that it's meant to directly consume EF context queries including in your web app layer. If the DbContext was meant to be shut away inside a library, they would have designed things a lot differently.

To sum up

I would recommend reconsidering the purpose of your generic repository, why you need it, and what it's buying you.

StackExchange Code Review Q#105454, answer score: 3