How to get EFC relationship fixup to work

[JasinCole]

I am trying to get the following code to work, but I do not believe I fully understand how this relationship fixup works. I want GetEmployeeRecordWithFilteredPayrecs() to return an employee with only the payrecs that belong to a certain payperiod.
I am sure this more of a "I do not understand" so a general explanation of the right approach and what needs to happen would probably benefit me most.

var payPeriods = new List<DateTime>
        {
            DateTime.Parse("11/6/2022"),
            DateTime.Parse("11/13/2022"),
            DateTime.Parse("11/20/2022"),
            DateTime.Parse("11/27/2022")
        };
var employeeNum = 1006;
var employee = srvc.GetEmployeeRecordWithFilteredPayrecs(employeeNum,
            payrec => payrec.Empnum == employeeNum && payPeriods.Contains((DateTime)payrec.Payprd!));

    public Employ? GetEmployeeRecordWithFilteredPayrecs(int employeeNum, Func<Payrec, bool> expression)
    {
        var employee = EmployeeRecordById(employeeNum);
        if (employee != default)
        {
            var _ = _ctx.Payrecs
                .Where(expression);
        }

        return employee;
    }

    private Employ? EmployeeRecordById(int employeeNum)
    {
        return _ctx.Employs
            .FirstOrDefault(employee => employee.Recnum == employeeNum);
    }

 

[cjard]

Looks to me like you're missing anything that will actually load the payrecs from the DB; a LINQ Where on its own does nothing without something to enumerate the resulting enumerable. If you want to load data from the DB but save the overhead of actually materializing it to something your code then throws away, you can call .Load rather than .ToList etc

Modern EFCore also does filtered include, so something like

  ctx.Employees
    .Include(e => 
      e.PayRecords.Where(pr => pr.Date >= new DateTime(2001,1,1) and pr.Date < new DateTime(2001,1,8))
    ).FirstOrDefault(e => e.Id == 123)

can work too

 

[JasinCole]

Let me ask this question, given the following code.
Is there any need to complicate this DAL any further by filtering collections attached to an employee record during db query or is it more logical to leave that logic to the DAL user?
Instinct tells me to filter the navigation property collections outside of the db query (as needed), but that doesn't make much sense overtime as the queries return more and more data (as pertaining to weekly pay records).
What's the prefered approach here when designing a DAL

using System.Linq.Expressions;
using DatabaseAccess.Entities;
using DatabaseAccess.Context;
using Microsoft.EntityFrameworkCore;

namespace DatabaseAccess;

public class EmployeeRepository
{
    private readonly SageDbContext _ctx;

    public EmployeeRepository(SageDbContext ctx)
    {
        _ctx = ctx;
    }

    public Employ? GetEmployeeRecordById(int employeeNum)
    {
        return EmployeeRecords(e => e.Recnum == employeeNum).FirstOrDefault();
    }

    public Employ? GetEmployeeRecordByName(string firstName, string lastName)
    {
        return EmployeeRecords(e => e.Fstnme == firstName && e.Lstnme == lastName).FirstOrDefault();
    }
    
    private IQueryable<Employ> EmployeeRecords(Expression<Func<Employ, bool>> employeePredicate)
    {
        return _ctx.Employs
            .Include(employ => employ.Payrecs)
            .Include(employ => employ.PaygrpNavigation)
            .Include(employ => employ.UninumNavigation)
            .Include(employ => employ.WrkcmpNavigation)
            .Where(employeePredicate);
    }
}

 

[cjard]

It depends (it always depends). Having a stack of includes like that might mean that your app constantly downloads huge amounts of data for trivial queries. Some newbie developer might be like "I need the employee status. I have the ID. I'll just call GetById" and they end up dragging 5000 other records from 4 related tables, every time the page loads.. Without even realizing because it's all hidden away in a nice "get by ID"

I don't think there is a stong solution for that, other than for devs to check they aren't over-requesting; a mandate of "use the context directly and query what you need in each different case" may lead to excessive repetition. An approach of "use the helper methods all the time" may over-request and "write one for every need" might lead to a messy proliferation. The general approach I see taken is to mix and balance these things out.

I wouldn't take the approach you have done, of passing a where clause, because it doesn't really save anything; have your common query that does the includes and return it, then do the where in the GetById on the returned queryable.

Actually you probably wouldn't even do a Where in the GetById; have it Whereless and use the fact that FirstOrDefault can take the predicate to find the single employee Id. This is perhaps a good example of why not to get into "pass a predicate that will always be where'd" - sometimes you don't want a where and by creating a method that demands one you make code harder to understand and you railroad your approach into being of a certain form. It also means that things end up looking a bit fragmented if your passed where predicate references other tables in a context

You might still be keen on it and then say "ok I'll make the predicate optional".. but then one day you have a need for a group by, or to select a different object so you start messing to add more optional arguments and passing mappers and making it generic..

And then you've ended up needlessly facading all the functionality EFCore provides anyway, and it'd be simpler and more consistent to throw it all away and use EFC directly for the flexibility it offers.. EFC already implements repository and unit of work, yet I see people looking to wrap it again to provide them (or to make it generic), and they usually end up hitting some artificial barrier the approach creates, then head straight to stack overflow for advice on how to solve the self imposed problem