I ask as I'm mostly self taught, and don't have a team to be able to get experience/challenge or ask.
The Requirement
I have a register per week. (ie the kind you mark attendance on), with sub sessions (known here as services). Ie, Morning, afternoon, evening etc. It is indicated whether someone is scheduled and/or whether they attended. (Someone can attend a different one to which they were scheduled.)
Coming from a UI, which posts a whole register; Into a API, through a BLL, into a repo, then a SQL Server database. A Register has attendees. Attendees have Services. Register → Attendees → Services.
Existing/Problematic
The existing method has issues, update & insert statements buried within two loops, which I know smells, hence I'm trying to remedy, but I'm not sure of the best way to do it.
Currently it was designed to…
- Loop through the Attendees;
- Then within that, loop through the Services.
- Then within that:
- Update the scheduled Service and
- Insert if someone attended a different Service.
With upto 50 people per register week, and typically 5 possible Services per day, a couple of database hits per one, we quickly have over 500 hits on the database per register save. This is obviously hitting the database a lot!
First thoughts
I'm trying to get my classes to adhere more to the SRP. What I was considering, in the BLL project, was this. (Where these are 2 lists of Services, the BLL getting the current register state, and the one submitted to the API call.) Which exposed something like:
public interface IRegisterChangeComparer
{
List<Service> GetAdded();
List<Service> GetRemoved();
List<Service> GetChangedServices();
}
public class RegisterChangeComparer : IRegisterChangeComparer
{
private List<Service> SubmittedServices;
private List<Service> ExistingServices;
public RegisterChangeComparer(List<Service> submittedServices, List<Service> existingServices)
{
SubmittedServices = submittedServices;
ExistingServices = existingServices;
}
public List<Service> GetAdded()
{
List<Service> newlyAdded = new List<Service>();
foreach (Service service in SubmittedServices.Where(s => !s.IsScheduled))
{
// Compare on Date, Service Type
if (!ExistingServices.Exists(e => e.ServiceDate == service.ServiceDate && e.TypeId == service.TypeId))
{
newlyAdded.Add(service);
}
}
return newlyAdded;
}
public List<Service> GetRemoved()
{
List<Service> removedServices = new List<Service>();
foreach (Service service in ExistingServices.Where(s => !s.IsScheduled))
{
// Compare on Date, Service Type
if (SubmittedServices.Exists(e => e.ServiceDate == service.ServiceDate && e.TypeId == service.TypeId))
{
removedServices.Add(service);
}
}
return removedServices;
}
public List<Service> GetChangedServices()
{
List<Service> changedServices = new List<Service>();
foreach (Service service in SubmittedServices)
{
//Find matching record from database data
Service matchedService = ExistingServices.FirstOrDefault(e => e.ServiceDate == service.ServiceDate && e.TypeId == service.TypeId);
if (matchedService == null) continue;
if (service.IsAttended != matchedService.IsAttended && service.IsScheduled)
{
changedServices.Add(service);
}
}
return changedServices;
}
}
Then pass that to the repo accordingly to save, but with grouped/less hits on the database.
Here is the Service Class. It probably ought to be using the Id for find/comparison.
public class Service
{
public int Id { get; set; }
public int ServiceUserId { get; set; }
public int SupplierId { get; set; }
public int TypeId { get; set; }
public DateTime ServiceDate { get; set; }
public decimal Units { get; set; }
public int PONumber { get; set; }
public decimal Rate { get; set; }
public bool IsScheduled { get; set; }
public bool IsAttended { get; set; }
public DateTime CreatedDate { get; set; }
public DateTime LastUpdatedeDate { get; set; }
}
Initially I'm after pointers to whether I'm going about it the right way or whether there's a better way to pursue. Any particular patterns to choose. I just want to improve and write better code.