Depending on the number of if
conditions, you could use the Strategy Pattern instead.
Something like this...(NOTE: This is not working code!)
public interface ICreateOrUpdateReportDefinition
{
void CreateOrUpdate(ReportGroupSubReport reportGroupSubReport, bool isNew);
bool ShouldHandle(int report);
}
public class CreateOrUpdateFactory
{
private ICreateOrUpdateReportDefinition[] _definitions;
public CreateOrUpdateFactory(ICreateOrUpdateReportDefinition[] definitions)
{
_definitions = definitions;
}
public void CreateOrUpdate(ReportGroupSubReport reportGroupSubReport, bool isNew, int report)
{
var definition = _definitions.FirstOrDefault(x => x.ShouldHandle(report));
if (definition != null)
definition.CreateOrUpdate(reportGroupSubReport, isNew);
}
}
public class CreateOrUpdateReportDefinitions : ICreateOrUpdateReportDefinition
{
var _envyUnitOfWork;
public CreateOrUpdateReportDefinitions(var envyUnitOfWork)
{
_envyUnitOfWork = envyUnitOfWork;
}
public void CreateorUpdate(ReportGroupSubReport reportGroupSubReport, bool isNew)
{
var entity = _envyUnitOfWork.ReportDefinitions.GetById(reportGroupSubReport.Id) ?? new ReportDefinition();
if (isNew)
entity.SetNew();
_envyUnitOfWork.ReportDefinitions.InsertOrUpdate(entity, true);
}
public bool ShouldHandle(int report)
{
return report == 1;
}
}
public class CreateOrUpdateTraxReports : ICreateOrUpdateReportDefinition
{
var _envyUnitOfWork;
public CreateOrUpdateTraxReports(var envyUnitOfWork)
{
_envyUnitOfWork = envyUnitOfWork;
}
public void CreateorUpdate(ReportGroupSubReport repotGrouSubReport, bool isNew)
{
var entity = _envyUnitOfWork.TraxReports.GetById(reportGroupSubReport.Id) ?? new TraxReport();
if (isNew)
entity.SetNew();
_envyUnitOfWork.TraxReports.InsertOrUpdate(entity, true);
}
public bool ShouldHandle(int report)
{
return report == 2;
}
}
You would setup the factory like this...
var envyUnitOfWork = new EnvyUnitOfWork(); // or however you create an instance of this...
var factory = new CreateOrUpdateFactory(new CreateOrUpdateReportDefinitions(envyUnitOfWork), new CreateOrUpdateTraxReports(envyUnitOfWork));
Then all you have to do is call it like this...
factory.CreateOrUpdate(reportGroupSubReport, isNew, report);
Final thoughts:
I like the Strategy Pattern when you have a number of conditions because it segregates the code into separate classes, making it easier to maintain and unit test.
Another nice thing about this approach is when new report definitions are added you just have to add a new class and pass that new class into the factory constructor. Nothing else changes which you are less likely to break code elsewhere.
I tend to pass classes that do not change into the constructor which is why I am passing _envyUnitOfWork
in the constructor. There are others who prefer to pass everything into the method. I think either way will work but wanted to give you the two schools of thought so you can decide for yourself.
ifs
for aswitch
statement and work with the calls toTraxReports
,ReportDefinitions
, etc as separated methods \$\endgroup\$ – Gerep Mar 28 '16 at 20:56report
parameter with an Enum first. Then see if both objects obtained by ID have a common denominator that supports all methods afterwards. And then you can start your refactoring \$\endgroup\$ – Kolky Mar 28 '16 at 21:12