C# DbContext with nested classes containing the Repository methods

Question

So I have done a lot of reading and it appears that, since DbContext already implements a repository and unit of work pattern combination, creating more abstraction classes would be redundant. However, I would still like to be able to encapsulate commonly-used queries for code reuse. I was thinking - perhaps I may be able to store a query-container class that uses the current instance of DbContext to make sure that queries don't cross transaction boundaries. Take for instance, this dummy code:

public partial class PersonEntities : DbContext
{
    public PersonEntities() : base("name=SomeConnectionString")
    {
        this.Configuration.ProxyCreationEnabled = false;
        this.CustomersQuery = new CustomerQueries(this);
    }

    public virtual DbSet<Customer> Customers { get; set; }
    public virtual DbSet<Employees> Employees { get; set; }

    public CustomerQueries CustomersQuery { get; }

    public class CustomerQueries 
    { 
        protected PersonEntities Database { get; private set; }

        internal CustomerQueries(PersonEntities db)
        {
            this.Database = db;
        }

        public IEnumerable<Customer> GetCustomersByName(string firstName, string lastName, bool ignoreCase = true)
        {
            /// method implementation
        }
    }
}

You might use it like this (concrete classes and lack of factories for simplification):

        using (var context = new PersonEntities())
        {
            var query = context.CustomersQuery.GetCustomersByName("Alfred", "Morris", true);

            /// stuff

            await context.SaveChanges();
        }

Pros:

1. Does not add additional abstraction
2. Queries can use multiple DbSets at once within the same transaction, unlike with many of the repository (especially generic repository) implementatinos I've seen.
3. Related queries are neatly contained in a single place with mostly only relevant methods (plus the common object methods like ToString), meanwhile DbSet class itself has tons of methods thanks to IEnumerable/IQueryable/etc extension methods and thus extending DbSet didn't seem like a good idea.
4. It also becomes possible to make some generic queries, such as: GetPersonbyName<TPerson>(string firstName, stringLastName) where TPerson : Person, perhaps contained in a PersonsQuery class.

Cons

1. Adds additional functionality that could be considered bloating, potentially leading to a 'god class' - however, these queries are still tightly coupled to the DbSets and are still in sub-classes so I'm not sure if this is the case.
2. Perhaps others? I'm not really sure.

Are there any other drawbacks to using this style for code-reuse without redundant abstractions?

Answer 1

For this sort of thing I like to use C# extension methods against IQueryable<>. You would still have a class do define these but it wouldn't be nested in or referenced from your context.

Example:

public static class IQueryableOfCustomerExtensions
{
    public static IQuerable<Customer> GetByName(this IQueryable<Customer> customers, string firstName, string lastName, bool ignoreCase = true)
    {
        // Implementation
    }
}

You could then call this like:

var customers = context.Customers.GetByName("Alfred", "Morris", true).ToList();

Answer 2

This is basically the repository pattern (at least how I interpret the repository pattern). I use this exclusively for all of my work with data sources so, personally, I think this is a great way to go.

That being said, I want to challenge an underlying statement in your question.

creating more abstraction classes would be redundant.

EF is only an abstraction over the SQL language. What it doesn't provide is an abstraction over your schema in the first place or even the source of data (stored in a database or a third party API?).

If you're developing anything more complicated than a simple TODO application, you may find a lot of value in abstracting these things out as well. In order to do that, I create separate domain models that I immediately convert to after getting query results back. Then, I use interface separation for all my repos. And the repos return my domain models, not my EF models (which I now just call DTOs and schema objects). This way, if I ever change my data source (which I have done) or if I'm unit testing and don't want the overhead of EF, the existance of EF is just a trivial detail. I can just stub out the repository interfaces now in tests or in production, I can just rewrite the repo without touching any business logic.

Example:

// Domain:

class User
{
    // This is my domain model
}

interface ICustomerRepository
{
    Task<Customer?> GetCustomerByName(string name);
}

// Data stuff

class CustomerDTO
{
    // This does not move beyond the repository implementation class. Doesn't even get exposed in the interface. It's just a detail of EF.
}

class PersonEntities : DbContext
{
    DbSet<CustomerDTO> Customers { get; set; }
}

The repository implementation (If you ever completely redo the way customers are stored... like say you move them to a separate API requiring an HTTP request instead of SQL... you simply rewrite the repo and you leave the business logic alone).

class CustomerRepository : ICustomerRepository
{
    private PersonEntities dbContext;

    public async Task<Customer> GetCustomerByName(string name)
    {
        CustomerDTO? customerDTO = await this.dbContext.Customers.Where(...).SingleOrDefaultAsync();

        if (customerDTO == null) return null;

        Customer domain = new Customer(customerDTO.Id, customerDTO.FirstName, customerDTO.LastName, ...);

        return domain;
    }
}

// Business

// I use dependency injection for these but you can always just new it up too... you just lose the dependency inversion that way.
private ICustomerRepository customerRepository;

// Some example use case in your application
public async Task CreateCustomer(string customerName)
{
    Customer? customer = await this.customerRepository.GetCustomerByName(customerName);
    
    if (customer != null)
    {
        throw new Exception("Customer with that name already exists.");
    }

    // ... the rest of the use case.
}