i.e. (from a Foo''''''Servlet''''''Command''''''Factory) This: if ( serviceName.equals( Foo''''''Command.WENTRY_COMMAND ) ) { newCommand = new WEntryCommand( servlet ); } else if ( serviceName.equals( Foo''''''Command.WGET_ESENSE_PREFS_COMMAND ) ) { newCommand = new WGetESensePrefsCommand( servlet ); } else if ( serviceName.equals( Foo''''''Command.WREGISTER_COMMAND ) ) { newCommand = new WRegisterCommand( servlet ); } else if ( serviceName.equals( Foo''''''Command.WGET_MEMBER_INFO_COMMAND ) ) { newCommand = new WGetMemberInfoCommand( servlet ); } else if ( serviceName.equals( Foo''''''Command.WSET_MEMBER_INFO_COMMAND ) ) { newCommand = new WSetMemberInfoCommand( servlet ); } else if ( serviceName.equals( Foo''''''Command.WREP_RESET_PASSWORD_COMMAND ) ) { newCommand = new WRepResetPasswordCommand( servlet ); } else if ( serviceName.equals( Foo''''''Command.WPURCHASE_COMMAND ) ) { newCommand = new WPurchaseCommand( servlet ); } else if ( serviceName.equals( Foo''''''Command.WSERVER_EXIT_COMMAND ) ) { newCommand = new WServerExitCommand( servlet ); } else if ( serviceName.equals( Foo''''''Command.WCLIENT_EXIT_COMMAND ) ) { newCommand = new WClientExitCommand( servlet ); } else if ( serviceName.equals( Foo''''''Command.WGENERATE_EXIT_PAGE_COMMAND ) ) { newCommand = new WGenerateExitPageCommand( servlet ); } else if ( serviceName.equals( Foo''''''Command.WUPDATE_MEMBERSHIP_COMMAND ) ) { newCommand = new WUpdateMembershipCommand( servlet ); } else if ( serviceName.equals( Foo''''''Command.WNON_RENEW_MEMBERSHIP_COMMAND ) ) { newCommand = new WNonRenewMembershipCommand( servlet ); } else if ( serviceName.equals( Foo''''''Command.WCANCEL_MEMBERSHIP_COMMAND ) ) { newCommand = new WCancelMembershipCommand( servlet ); } else if ( serviceName.equals( Foo''''''Command.WDISPLAY_TRANSCRIPT_IDS_COMMAND ) ) { newCommand = new WDisplayTranscriptIdsCommand( servlet ); } else if ( serviceName.equals( Foo''''''Command.WDISPLAY_TRANSCRIPT_TEXT_COMMAND ) ) { newCommand = new WDisplayTranscriptTextCommand( servlet ); } else if ( serviceName.equals( Foo''''''Command.WAUTHENTICATE_MEMBER_COMMAND ) ) { newCommand = new WAuthenticateMemberCommand( servlet ); } else if ( serviceName.equals( Foo''''''Command.WCLIENT_ENTRY_INSTRUCTION_COMMAND ) ) { newCommand = new WClientEntryInstructionCommand( servlet ); } else if ( serviceName.equals( Foo''''''Command.WCLIENT_ENTRY_CHAT_COMMAND ) ) { newCommand = new WClientEntryChatCommand( servlet ); } else if ( serviceName.equals( Foo''''''Command.WGET_ORDER_HISTORY_COMMAND ) ) { newCommand = new WGetOrderHistoryCommand( servlet ); } else if ( serviceName.equals( Foo''''''Command.WREP_INFO_COMMAND ) ) { newCommand = new WRepSessionInfoCommand( servlet ); } else if ( serviceName.equals( Foo''''''Command.WINSTANT_MEMBER_ENTRY_COMMAND ) ) { newCommand = new WInstantMemberEntryCommand( servlet ); } else if ( serviceName.equals( Foo''''''Command.WFREE_HELP_COMMAND ) ) { newCommand = new WFreeHelpCommand( servlet ); } else if ( serviceName.equals( Foo''''''Command.WCOOKIE_BASED_ENTRY_COMMAND ) ) { newCommand = new WCookieBasedEntryCommand( servlet ); } else if ( serviceName.equals( Foo''''''Command.WREP_SESSION_INFO_COMMAND ) ) { newCommand = new WRepSessionInfoCommand( servlet ); } else if ( serviceName.equals( Foo''''''Command.WCHAT_POPUP_COMMAND ) ) { newCommand = new WChatPopupCommand( servlet ); } else if ( serviceName.equals( "WGetURLCommand" ) ) { newCommand = new WGetURLCommand( servlet ); } else if ( serviceName.equals( "WLogCommand" ) ) { newCommand = new WLogCommand( servlet ); } else if ( serviceName.equals( "WClientQueueEntryCommand" ) ) { newCommand = new WClientQueueEntryCommand( servlet ); } else if ( serviceName.equals( "WGetInflatedTokensCommand" ) ) { newCommand = new WGetInflatedTokensCommand( servlet ); } else { throw new Runtime''''''Exception("Unimplemented command: \"" + serviceName + "\"" ); } newCommand.init( req ); return newCommand; Turned into: /** * Handle WGetRepInfoCommand separately, since it doesn't map from string to classname. **/ if ( serviceName.equals( Foo''''''Command.WREP_INFO_COMMAND ) ) { newCommand = new WRepSessionInfoCommand( servlet ); } else { try { /** * Create a class of the name (with the appropriate package name tacked on the front. **/ Class commandClass = Class.forName( "com.foo.servlet." + serviceName ); /** * This step is necessary because the Foo''''''Commands don't have a default constructor, * so we have to get the appropriate constructor, and then call it's newInstance. **/ Constructor constructor = commandClass.getConstructor( new Class[]{ GenericServlet.class } ); newCommand = (Foo''''''Command) constructor.newInstance( new Object[]{ servlet } ); } catch( Exception exception ) { throw new Runtime''''''Exception("Unimplemented command: \"" + serviceName + "\"" ); } } newCommand.init( req ); return newCommand; Which is slower, but could be made just as fast with a Constructor-caching mechanism, and has the added benefit of not needing modification every time we want to add new commands. -- BlakeWinton See also CollectionAndLoopVsSelectionIdiom, PolymorphismVsSelectionIdiom This is an example of FactoryMap pattern. Normally, programmers have to register their classes with the factory, but in Java the class loader does it for you. -- HubertMatthews ''Don't try that on JavaCard, though.'' You could also try a factory map something like this: cmd = CommandManager.getInstance().createFrom( serviceName ); ... interface CommandFactory { Command create(); } ... // in some initializer CommandManager.getInstance().map( WREP_INFO_COMMAND, new CommandFactory() { public Command create() { return WRepSessionInfoCommand(); } } ); public class CommandManager { private HashMap m_factoryMap = ...; public void map( Object key, CommandFactory creator ) { m_factoryMap.put( key, creator ); } public Command createFrom( Object key ) { return (Command)((CommandCreator)m_factoryMap.get( key )); } ... } Or something like that. -- RobertDiFalco ''The point (well, at least one point) of the above is that you have one and only one easy to change place where it's defined which commands exist - in this case, through the existence of a class in the com.foo.servlet package. When you put a mapping into the database, you duplicate that information. What's worse, you put that information into two different "media" (db and classes), where they can more easily get out of synch. For updating that table, you might have to fire up another tool. Plus, it's harder to implement, and would have to be changed for implementations without a database. You put application-specific information into the database, which might be used for several applications. Etc. etc. All in all, it seems like the worst solution by far to me. Sorry. -- FalkBruegmann'' ---- See NestedCaseStatements, SwitchStatementsSmell ------- CategoryConditionalsAndDispatching ---- CategoryJava