Refactoring Java factory method -
there's unsatisfactory code:
/* given command string in first 8 characters command name padded on right whitespace, construct appropriate kind of command object. */ public class commandfactory { public command getcommand(string cmd) { cmdname = cmd.substring(0,8).trim(); if(cmdname.equals("start")) { return new startcommand(cmd); } if(cmdname.equals("end")) { return new endcommand(cmd); } // ... more commands in more if blocks here // else it's bad command. return new invalidcommand(cmd); } }
i'm unrepentant multiple exit points - structure clear. i'm not happy series of near-identical if statements. i've considered making map of strings commands:
commandmap = new hashmap(); commandmap.put("start",startcommand.class); // ... etc.
... using reflection make instances of appropriate class looked map. while conceptually elegant, involves fair amount of reflection code whoever inherits code might not appreciate - although cost might offset benefits. lines hardcoding values commandmap smell bad if block.
even better if factory's constructor scan classpath subclasses of command, query them string representations, , automatically add them them repertoire.
so - how should go refactoring this?
i guess of frameworks out there give me kind of thing free. let's assume i'm not in position migrate stuff such framework.
your map of strings commands think good. factor out string command name constructor (i.e. shouldn't startcommand know command "start"?) if this, instantiation of command objects simpler:
class c = commandmap.get(cmdname); if (c != null) return c.newinstance(); else throw new illegalargumentexception(cmdname + " not valid command");
another option create enum
of commands links classes (assume command objects implement commandinterface
):
public enum command { start(startcommand.class), end(endcommand.class); private class<? extends commandinterface> mappedclass; private command(class<? extends commandinterface> c) { mappedclass = c; } public commandinterface getinstance() { return mappedclass.newinstance(); } }
since tostring of enum name, can use enumset
locate right object , class within.
Comments
Post a Comment