I am trying to replace the simple factory StatsCreatorFactory.java class in order to delete the stink multiple use of switch case statements. This is my situation:
StatsServlet.java
public class StatsServlet extends HttpServlet{
private static final long serialVersionUID = 1L;
protected void processRequest(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException {
StatsContext context = new StatsContext(request,response);
**IStatsCreator creator = StatsCreatorFactory.getCreator(context);**
IChart chart = creator.createChart();
String jsonChart = creator.chartToJson(chart);
creator.sendResponse(jsonChart);
}
@Override
protected void doPost(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException {
processRequest(request, response);
}
}
IStatsCreator.java
public interface IStatsCreator {
public IChart createChart() throws IOException;
public IDetailsTable createDetailsTable(String itemSelected);
public String chartToJson(IChart chart);
public String tableToJson(IDetailsTable table);
public void sendResponse(String resp) throws IOException;
public List<File> findFiles() throws IOException, ParseException;
public List<LogLine> parseFiles(List<File> files) throws IOException;
public IFileIntervalDateDetector getFileDetector() throws IOException;
public TargetChartOperation getTargetOperation();
public IChart getChart();
public IDetailsTable getDetailsTable();
}
AbstractStatsCreator
public abstract class AbstractStatsCreator implements IStatsCreator{
protected StatsContext context;
public AbstractStatsCreator(StatsContext context) {
this.context = context;
}
protected abstract ILogParser getParser();
protected abstract IStatsHelper getHelper();
@Override
public IFileIntervalDateDetector getFileDetector() throws IOException {
IFileIntervalDateDetector fileDetector = new FileDetector();
fileDetector.addPattern(new FileNamePattern(TypeSubjectEnum.valueOf(context.getSubject().toUpperCase()).getFilePattern()));
fileDetector.addPattern(new FileNamePattern(context.getInstance()));
return fileDetector;
}
@Override
public final List<File> findFiles() throws IOException, ParseException{
if(context.getDateStart().equalsIgnoreCase(StringUtils.EMPTY) && context.getDateEnd().equalsIgnoreCase(StringUtils.EMPTY)){
return getFileDetector().findDailyFiles();
}
Date startDate = new SimpleDateFormat("ddMMyyyy").parse(context.getDateStart());
Date stopDate = new SimpleDateFormat("ddMMyyyy").parse(context.getDateEnd());
Date currentDate = new Date(System.currentTimeMillis());
if(DateUtils.isSameDay(startDate, stopDate) && DateUtils.isSameDay(startDate, currentDate)){
return getFileDetector().findDailyFiles();
}
return getFileDetector().findFilesByInterval(context.getDateStart(), context.getDateEnd());
}
@Override
public final List<LogLine> parseFiles(List<File> files) throws IOException{
return getParser().parseLogFiles(files);
}
@Override
public IChart createChart() throws IOException{
if(context.needUpdate()){
List<File> files = null;
try {
files = findFiles();
} catch (ParseException e) {
files=Lists.newArrayList();
}
List<LogLine> logLines = parseFiles(files);
context.setLogLines(logLines);
context.updateContext();
}
IChart chart = getChart().create();
return chart;
}
@Override
public IDetailsTable createDetailsTable(String itemSelected) {
IDetailsTable table = getDetailsTable().create(itemSelected);
return table;
}
@Override
public String chartToJson(IChart chart) {
StringBuilder json = new StringBuilder(JsonTransformer.renderChart(chart));
return json.toString();
}
@Override
public String tableToJson(IDetailsTable table) {
StringBuilder json = new StringBuilder(JsonTransformer.renderDetailsTable(table));
return json.toString();
}
@Override
public void sendResponse(String resp) throws IOException {
context.getResponse().setContentType("application/json");
PrintWriter out = context.getResponse().getWriter();
out.write(resp.toString());
out.flush();
}
}
StatsCreatorFactory.java
public class StatsCreatorFactory {
public static IStatsCreator getCreator(StatsContext context){
if(context == null){
throw new IllegalArgumentException("Context nullo");
}
IStatsCreator creator=null;
switch (context.getOperation()) {
case "validate":
creator = new ValidateStatsCreator(context);
break;
case "extract":
creator = new ExtractStatsCreator(context);
break;
case "transform":
creator = new TransformStatsCreator(context);
break;
case "view":
creator = new ViewStatsCreator(context);
break;
default:
creator = new GeneralStatsCreator(context);
break;
}
return creator;
}
}
I would try to find a way to instantiate ICreator classes avoiding simple factory class, is there any refactoring or design pattern could I use? Reading Martin Fawler's book I wonder if I can use polymorphism, but I can't find any way to replicate it in my code.
I would first try to modify the IStatsCreator and AbstractStatsCreator to have a stateless bean.
In your example, you have only to get rid of the StatsContext context defined as a class variable in AbstractStatsCreator. So the context should not be bound to the class instance. It should be passed in from outside when ever a method is called on the creator that needs the context. For that you can refactor your IStatsCreator and add the context to all methods that needs it.
for example:
public IChart createChart() throws IOException;
new:
public IChart createChart( StatsContext context ) throws IOException;
and so on. After that you don't have to create new instances of AbstractStatsCreator implementations for every context call. you just need to have an instance per type. This instances of type can be mapped in the StatsCreatorFactory and just get when they needed. I would also recomend to ged rid of static methods. Make StatsCreatorFactory a real bean that can easier be managed and also easier be mocked for tests.:
public class StatsCreatorFactory {
private Map<String, IStatsCreator> statsCreators = new HashMap<String, IStatsCreator>();
public void registerStatsCreator( String type, IStatsCreator creator ) {
statsCreators.put( type, creator );
}
public IStatsCreator getCreator( String type ){
IStatsCreator creator= statsCreators.get( type );
if(creator == null){
throw new IllegalArgumentException("no creator registered for type : " + type);
}
return creator;
}
}
At the end StatsCreatorFactory is more like a provider then a factory. Maybe you can also rename it to StatsCreatorProvider.
public class StatsServlet extneds HttpServlet{
private static final long serialVersionUID = 1L;
private StatsCreatorProvider statsCreatorProvider;
public void init() {
statsCreatorProvider = new StatsCreatorProvider();
statsCreatorProvider.registerStatsCreator( "validate", new ValidateStatsCreator() );
statsCreatorProvider.registerStatsCreator( "extract" new ExtractStatsCreator() );
...
}
protected void processRequest(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException {
StatsContext context = new StatsContext(request,response);
IStatsCreator creator = statsCreatorProvider.getCreator( context.getOperation() );
IChart chart = creator.createChart( context );
String jsonChart = creator.chartToJson(chart);
creator.sendResponse(jsonChart);
}
@Override
protected void doPost(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException {
processRequest(request, response);
}
}