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.


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; }


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

        // 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.

  • 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, 2020 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\$ Oct 12, 2020 at 8:30
  • 4
    \$\begingroup\$ WorkShiftRepository is not a repository, it's a service. \$\endgroup\$ Oct 12, 2020 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, 2020 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, 2020 at 10:26


Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Browse other questions tagged or ask your own question.