Problem
In Service.Insert()
, I call DeviceRepo.Insert()
and in Service.Update()
, I call DeviceRepo.Update()
. All my other code is exactly the same.
I’m thinking I can make one method that do the same thing update. I want to update and insert when I will insert a device.
The methods respectively insert a new or update an existing (IoT) device into my database with entity framework 6.
How could I refactor the code? You can see I have the same methods twice where only another method is called.
public class DeviceService : IDeviceService
{
internal IGenericRepo<Device> _deviceRepo;
public DeviceService()
{
_deviceRepo = new DeviceRepo(); // P.S.: DeviceRepo overrides GenericRepo<Device> who implements IGenericRepo<Device>
}
public void Insert(Device device, IEnumerable<int> selectedFrameworks, IEnumerable<int> selectedOSs)
{
device.Framework = GiveObjects<Framework>(selectedFrameworks, _frameworkRepo);
device.OS = GiveObjects<OS>(selectedOSs, _osRepo);
SetStateUnchanged<Framework>(device.Framework);
SetStateUnchanged<OS>(device.OS);
_deviceRepo.Insert(device);
_deviceRepo.SaveChanges();
}
public void Update(Device device, IEnumerable<int> selectedFrameworks, IEnumerable<int> selectedOSs)
{
device.Framework = GiveObjects<Framework>(selectedFrameworks, _frameworkRepo);
device.OS = GiveObjects<OS>(selectedOSs, _osRepo);
SetStateUnchanged<Framework>(device.Framework);
SetStateUnchanged<OS>(device.OS);
_deviceRepo.Update(device);
_deviceRepo.SaveChanges();
}
}
GenericRepo<Device>
:
public class GenericRepo<TEntity> : IGenericRepo<TEntity> where TEntity : class
{
internal ApplicationDbContext context;
internal DbSet<TEntity> dbSet;
public GenericRepo()
{
context = new ApplicationDbContext();
dbSet = context.Set<TEntity>();
}
public virtual TEntity Insert(TEntity entity)
{
return dbSet.Add(entity);
}
public virtual void Update(TEntity entityToUpdate)
{
context.Entry(entityToUpdate).State = EntityState.Modified;
dbSet.Attach(entityToUpdate);
}
public virtual void SaveChanges()
{
context.SaveChanges();
}
}
public class DeviceRepo : GenericRepo<Device>
{ /* not important methods I override for this question */ }
Solution
I’m hesitant to post this as an answer as I’m not sure how I feel about it from a design perspective. But it’s certainly one way to reduce the repetition.
public class DeviceService : IDeviceService
{
internal IGenericRepo<Device> _deviceRepo;
public DeviceService()
{
_deviceRepo = new DeviceRepo(); // P.S.: DeviceRepo overrides GenericRepo<Device> who implements IGenericRepo<Device>
}
public void Insert(Device device, IEnumerable<int> selectedFrameworks, IEnumerable<int> selectedOSs)
{
DoXXX(device, selectedFrameworks, selectedOSs, _deviceRepo.Insert);
}
public void Update(Device device, IEnumerable<int> selectedFrameworks, IEnumerable<int> selectedOSs)
{
DoXXX(device, selectedFrameworks, selectedOSs, _deviceRepo.Update);
}
//Obviously call this something that makes sense
private void DoXXX(Device device, IEnumerable<int> selectedFrameworks, IEnumerable<int> selectedOSs, Action<Device> deviceAction)
{
device.Framework = GiveObjects<Framework>(selectedFrameworks, _frameworkRepo);
device.OS = GiveObjects<OS>(selectedOSs, _osRepo);
SetStateUnchanged<Framework>(device.Framework);
SetStateUnchanged<OS>(device.OS);
deviceAction(device);
_deviceRepo.SaveChanges();
}
}
I think in this case I would simply make a private method to capture the duplicated code. I probably wouldn’t mind about the extra call to the SaveChanges() as it’s only one line and removes any need for further complexity on the code.
public void Insert(Device device, IEnumerable<int> selectedFrameworks, IEnumerable<int> selectedOSs)
{
UpdateDeviceProperties(device, selectedFrameworks, selectedOSs);
_deviceRepo.Insert(device);
_deviceRepo.SaveChanges();
}
public void Update(Device device, IEnumerable<int> selectedFrameworks, IEnumerable<int> selectedOSs)
{
UpdateDeviceProperties(device, selectedFrameworks, selectedOSs);
_deviceRepo.Update(device);
_deviceRepo.SaveChanges();
}
private void UpdateDeviceProperties(Device device, IEnumerable<int> selectedFrameworks, IEnumerable<int> selectedOSs)
{
device.Framework = GiveObjects<Framework>(selectedFrameworks, _frameworkRepo);
device.OS = GiveObjects<OS>(selectedOSs, _osRepo);
SetStateUnchanged<Framework>(device.Framework);
SetStateUnchanged<OS>(device.OS);
}
I might also consider returning either a boolean (or some other error result) from the methods to raise awareness of whether the db action actually succeeded to help with raising failures back to the caller of the method.
In the case of Insert it could be as simple as (assuming device has an Id of course) return device.Id > 0;