4
\$\begingroup\$

I have two entities in my database: Employee and Shift. I want to implement a C# repository pattern method, that given two workshift IDs, swaps the two workshifts for two employees.

Limitation: one employee cannot work multiple shifts at a time.

Models

public class Employee
{
    public int ID { get; set; }
    public string Firstname { get; set; }
    public string Lastname { get; set; }
    public string Email { get; set; }
}

public class WorkShift
{
    public int ID { get; set; }
    public int EmployeeID { get; set; }
    public DateTime ShiftStart { get; set; }
    public DateTime ShiftEnd { get; set; }
}

Repository

public class WorkShiftRepository : RepositoryBase<WorkShift>, IWorkShiftRepository
{
    private readonly ShiftContext _databaseContext;

    public WorkShiftRepository(ShiftContext context) : base(context)
    {
        _databaseContext = context;
    }

    private async Task<bool> Overlaps(WorkShift shift, int employeeId)
    {
        return await _databaseContext.Shifts.Where(u => u.EmployeeID == employeeId).
            Where((s => shift.ShiftStart < s.ShiftEnd && s.ShiftStart < shift.ShiftEnd)).AnyAsync();
    }

    public async Task<int> SwapShifts(int shiftIdA, int shiftIdB)
    {
        var result = 0;
        // Load shifts into memory
        var shiftA = await _databaseContext.Shifts.SingleAsync(s => s.ID == shiftIdA);
        var shiftB = await _databaseContext.Shifts.SingleAsync(s => s.ID == shiftIdB);
        int shiftAid = shiftA.EmployeeID;

        // Swap Employee IDs
        shiftA.EmployeeID = shiftB.EmployeeID;
        shiftB.EmployeeID = shiftAid;

        // Check if the swapped shifts has any overlaps on db
        if (await Overlaps(shiftA, shiftA.EmployeeID) || await Overlaps(shiftB, shiftB.EmployeeID))
            throw new Exception("ShiftsOverlap");

        // update in memory
        _databaseContext.Shifts.Update(shiftA);
        _databaseContext.Shifts.Update(shiftB);

        // Write changes to DB
        result = await _databaseContext.SaveChangesAsync();

        return result;
    }
}

The code compiles and it meets the limitations. But I am looking for tips and tricks to make the code cleaner and/or more efficient.

\$\endgroup\$
5
  • 1
    \$\begingroup\$ not worth an answer, but throw new Exception("ShiftsOverlap"); I feel return await Task.FromResult(-1); would be better, because you don't need to throw an exception just because it's overlapped ! you just need to stop updating the swapping. Plus, you need to add validations such as null validations. \$\endgroup\$ – iSR5 Oct 11 '20 at 16:10
  • \$\begingroup\$ @JensJensen I think there is a problem with your code. The compare and swap methods are not treated as atomic. So it is possible that between checking overlap and actually performing the interchange one of employees modifies his/her shift and the overlap check become invalid. \$\endgroup\$ – Peter Csala Oct 12 '20 at 8:30
  • 4
    \$\begingroup\$ WorkShiftRepository is not a repository, it's a service. \$\endgroup\$ – Gert Arnold Oct 12 '20 at 8:32
  • 1
    \$\begingroup\$ shiftAid is badly named. It reads as "aid" not "a-id", plus it is the employee id, not the id of "shift-a". \$\endgroup\$ – BCdotWEB Oct 12 '20 at 10:22
  • 1
    \$\begingroup\$ You can one line swap with tuples: (shiftA.EmployeeID, shiftB.EmployeeID) = (shiftB.EmployeeID, shiftA.EmployeeID);. You could argue it's a terrible thing to do but I like it. \$\endgroup\$ – RobH Oct 12 '20 at 10:26

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Browse other questions tagged or ask your own question.