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 workshift
s 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.
throw new Exception("ShiftsOverlap");
I feelreturn 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\$WorkShiftRepository
is not a repository, it's a service. \$\endgroup\$shiftAid
is badly named. It reads as "aid" not "a-id", plus it is the employee id, not the id of "shift-a". \$\endgroup\$(shiftA.EmployeeID, shiftB.EmployeeID) = (shiftB.EmployeeID, shiftA.EmployeeID);
. You could argue it's a terrible thing to do but I like it. \$\endgroup\$