Splitting Linq Query across Multiple Functions

Question

I have the following Linq Query operation (Simplified version) which is contained in a single function

    var emp = from e in context.DB_Employee
              .Skip(0) 
              .Take(10)
          select new Employee
          {
              id = e.Id,
              name = e.Name
              address = context.DB_Address.Where(a => a.Id == e.Id)
                  .Select(a => new Address
                  {
                      houseNumber = a.HouseNumber
                      street = a.Street
                      country = a.Country
                  }).ToArray(),
                  
                  
              benefits = context.DB_Benefits.Where(b => b.Id == e.Id)
                  .Select(b => new EmployeeBenefits
                  {
                      startDate = b.StartDate
                      expiryDate = b.ExpiryDate
                  }).ToArray(),
          }

Calling the function will return a result like the below

  {
    "Employee": {
        "id ": 1,
        "name ": "John",
        "address": {
            "houseNumber": "20",
            "street": "street_1",
            "country": "country_1"
        },
        "benefits": {
            "startDate": "2020-01-01",
            "expiryDate": "2020-01-01"
        }
    }
}

I would like to refactor this single function into multiple functions so I have the flexibility to choose the bits to add to the final output. The functions would be layed out something like this

public IEnumerable<Employee> GetEmployee()
{
...
AddAddress();
AddBenefits();
...
}

Answer 1

The first question would be "Why?" Why do you want to split up the data retrieval across multiple methods and potentially make it conditional? It is certainly possible, but you are taking a fairly efficient query and will be replacing it with several less efficient queries.

Looking at your example, the first thing I see missing are navigation properties for the relationships between Employee, Address, and Benefits. From reading the relationships I would expect to see something like:

public class Employee
{
    [Key]
    public int EmployeeId { get; set; }
    public string Name { get; set; }
    // ...

    public virtual ICollection<Address> Addresses { get; set; } = new List<Address>();
    public virtual ICollection<Benefit> Benefits { get; set; } = new List<Benefit>();
}

... then to query...

var employees = context.DB_Employee
    .Select(x => new Employee
          {
              id = e.Id,
              name = e.Name
              addresses = e.Addresses
                  .Select(a => new Address
                  {
                      houseNumber = a.HouseNumber
                      street = a.Street
                      country = a.Country
                  }).ToArray(),
              benefits = e.Benefits
                  .Select(b => new EmployeeBenefits
                  {
                      startDate = b.StartDate
                      expiryDate = b.ExpiryDate
                  }).ToArray(),
          })
    .Skip(pageNumber * pageSize)
    .Take(pageSize)
    .ToArray();

Libraries like Automapper can simplify this even further with ProjectTo where you configure Automapper with how to translate Entity to ViewModel, and it does the rest as opposed to the various Select statements.

var employees = context.DB_Employee
    .ProjectTo<Employee>(config)
    .Skip(pageNumber * pageSize)
    .Take(pageSize)
    .ToArray();

... where config is the Automapper config for mapping. (not shown, check Automapper documentation for examples)

From there, the question is Why would this need to be split up? We've got an Employee(ViewModel?) which should reflect the type of data the view is expecting. It's generally not a good idea to conditionally populate data in a model otherwise the consumers of that model will have to inspect the model to somehow determine whether it's complete enough for them. (Do I expect Addresses or not? etc.) If I did want to introduce conditional details then I would look at using separate view models. For example if I wanted to return just employee details vs. employee details /w address & benefits:

if (includeDetails)
    return  context.DB_Employee
        .ProjectTo<EmployeeWithDetails>(config)
        .Skip(pageNumber * pageSize)
        .Take(pageSize)
        .ToArray();
else
    return  context.DB_Employee
        .ProjectTo<EmployeeSummary>(config)
        .Skip(pageNumber * pageSize)
        .Take(pageSize)
        .ToArray();

Where I define different view models (EmployeeWithDetails and EmployeeSummary) with the appropriate mapping rules, then depending on the conditional logic, populate one or the other. This could be done with switch/case etc. I would avoid conditionally appending fields to a single model:

var employees = context.DB_Employee
    .ProjectTo<Employee>(config)
    .Skip(pageNumber * pageSize)
    .Take(pageSize)
    .ToArray();

if (includeAddresses)
{
    foreach(var employee in employees)        
        employee.Addresses = context.DB_Addresses
            .Where(x => x.EmployeeId == employee.Id)
            .ProjectTo<Address>()
            .ToArray();
}

if (includeBenefits)
{
    foreach(var employee in employees)        
        employee.Benefits = context.DB_Benefits
            .Where(x => x.EmployeeId == employee.Id)
            .ProjectTo<Benefit>()
            .ToArray();
}

The problems with this approach are that consumers expecting "Employee" models may or may not get addresses and/or benefits. There is also the issue of extra querying to get the conditional data: Like other examples such as:

select new Employee
{
    id = e.Id,
    name = e.Name
    address = _addressService.GetAddress(context),    
    benefits = _benefitsService.GetBenefits(context)
}

The issue here is that you are querying the DB for the Address and again for the Benefit for each and every employee you load, rather than loading them as part of the Employee load.

So to fetch 10 employees where you also want address and benefits, you're executing 21 queries instead of 1.

Answer 2

Refactored Code: Address and EmployeeBenefit implemented from other function and called both the function in employee LINQ query

public IEnumerable<Employee> GetEmployee()
{  
    var emp = context.DB_Employee.AsEnumerable().Skip(0).Take(10)
                select (e=>new Employee
                      {
                          id = e.Id,
                          name = e.Name,
                          address = GetAddress(context),
                          EmployeeBenefits=GetEmployeeBenefits(context)
                }).ToList();
}
        

Address Function

public List<Address> GetAddress(DBContext _context)
{
     return _contextDB_Address.Where(a => a.Id == e.Id)
                  .Select(a => new Address
                  {
                      houseNumber = a.HouseNumber
                      street = a.Street
                      country = a.Country
                  }).ToList(),
}

EmployeeBenefits function

public List<EmployeeBenefits> GetEmployeeBenefits(DBContext _context)
{
     return _context.DB_Benefits.Where(b => b.Id == e.Id)
                  .Select(b => new EmployeeBenefits
                  {
                      startDate = b.StartDate
                      expiryDate = b.ExpiryDate
                  }).ToList(),
}