2009-10-19 7 views
2
public void handleParsedCommand(String[] commandArr) { 
    if(commandArr[0].equalsIgnoreCase("message")) { 
     int target = Integer.parseInt(commandArr[1]); 
     String message = commandArr[2]; 
     MachatServer.sendMessage(target, this.conId, message); 
    } else if(commandArr[0].equalsIgnoreCase("quit")) { 
     // Tell the server to disconnect us. 
     MachatServer.disconnect(conId); 
    } else if(commandArr[0].equalsIgnoreCase("confirmconnect")) { 
     // Blah blah and so on for another 10 types of command 
    } else { 
     try { 
      out.write("Unknown: " + commandArr[0] + "\n"); 
     } catch (IOException e) { 
      System.out.println("Failed output warning of unknown command."); 
     } 
    } 
} 

J'ai cette partie de mon code de serveur pour gérer les types de messages. Chaque message contient le type dans commandArr[0] et les paramètres dans le reste de commandArr[]. Cependant, ce code actuel, tout en travaillant, semble très peu éloquent. Y a-t-il une meilleure façon de le gérer? (Au meilleur de ma connaissance, String valeurs ne peuvent pas être utilisées dans switch déclarations, et même alors, une déclaration switch ne serait qu'une petite amélioration.Meilleur moyen d'écrire ce code Java?

+0

Quelle version de Java? SWITCH est * enfin * supporté par une chaîne dans la version la plus récente (ou a été publiée?) –

+0

C'est Java 6. . – Macha

+0

peut-être http://stackoverflow.com/questions/1199646/long-list-of-if-statements-in-java/1199677 est lié à celui-ci? – dfa

Répondre

8

Je aurait refactorisons cela en utilisant le Command Design Pattern.

fondamentalement, chacun de vos commandes, un message, quittez, confirmconnect et un défaut aura une implémentation de classe et mettra en œuvre l'interface de commande.

/*the Command interface*/ 
public interface ICommand { 
    void execute(String[] commandArr); 
} 

public class Message implements ICommand { 
    void execute(String[] commandArr) { 
     int target = Integer.parseInt(commandArr[1]); 
     String message = commandArr[2]; 
     MachatServer.sendMessage(target, this.conId, message); 
    } 
} 

//same type of class for other commands 

public class CommandManager { 
    public ICommand getCommand(String commandName) { 
     //store all the commands in a hashtable. 
     //Pull them out by name as requested. 
    } 

    //Esko's suggestion from comments 
    public static void executeImmediately(String[] commandArr) { 
     getCommand(commandArr[0]).execute(commandArr); 
    } 
} 


public void handleParsedCommand(String[] commandArr) { 

    ICommand command = CommandManager.getCommand(commandArr[0]); 
    command.execute(commandArr); 

//or Esko 

    CommandManager.executeImmediately(commandArr); 

} 
+0

Je me demande si vous ne décrivez pas le modèle de stratégie ici plutôt que le modèle de commande? –

+0

Les stratégies doivent pouvoir se substituer les unes aux autres; ce sont des méthodes alternatives de faire la même chose. Je ne pense pas que cela puisse être considéré comme une application du modèle de stratégie. – jprete

+2

Les stratégies décrivent des algorithmes enfichables. Par exemple, le tri, vous pouvez avoir un QuickSortStrategy ou un BubbleSortStrategy. Le résultat final de l'un ou l'autre serait le même, une liste triée. Mais comment cela se fait est différent. Les commandes encapsulent une seule action. – Bob

1

Jetez un oeil à Commons CLI qui est un analyseur d'arguments de ligne de commande.

Voici some examples de son utilisation.

+0

Ce ne sont pas des arguments de ligne de commande. Ils sont du réseau. – Macha

+0

Source n'a pas d'importance, vraiment. CLI prend 'String []' comme entrée. – ChssPly76

1

Vous pouvez utiliser enums

1

Pour commencer, je voudrais faire une carte entre les commandes et une classe qui exécute chaque type de commande (par exemple une classe anonyme qui implémente une interface connue), puis récupérer la bonne classe de la carte, puis passe le reste des paramètres. Si cela avait du sens, vous pourriez utiliser une énumération ici avec une méthode statique pour récupérer la bonne, de cette façon vous pourriez changer si et quand vous le deviez (disons que vous deviez faire la même chose sur 5 des 10 commandes).

0

Tout d'abord, vous lisez le même élément du tableau à chaque fois. Cela devrait être la première chose à prendre en compte. equalsIgnoreCase est un peu long, donc normalisez d'abord le cas (ne prenez pas les paramètres régionaux par défaut!).

Il est possible d'utiliser enum s pour pirater un switch de Swting s. JDK7 peut inclure un switch sur String, IIRC.

0

J'aime la réponse de Bob. Une autre méthode consisterait à utiliser le framework Spring et la fonctionnalité IoC. Fondamentalement, je l'ai fait avant d'utiliser Spring (gonfle de xml) pour créer une carte où vous avez chaque objet de commande stocké avec une clé. La clé serait la même que le texte en commandArr[0].

Alors votre xml ressemble à quelque chose comme

<property name="commands"> 
    <map> 
    <entry> 
     <key> 
      <value>message</value> 
     </key> 
     <ref bean="messageCommand" /> 
    </entry> 
    </map> 
</property> 
<bean id="messageCommand" ref="org.yourorg.yourproject.MessageCommand" /> 

Et puis dans votre code ...

commands.get(commandArr[0]).execute() //or whatever 

Cela vous permet de ne pas exécuter toute sorte de code d'initialisation. Tout ce que vous avez à faire est de gonfler le xml. Les poignées de printemps peuplent la carte pour vous. En outre, vous pouvez définir tous les membres de données nécessaires dans les classes en utilisant une syntaxe similaire. En outre, si vous avez besoin d'ajouter des fonctionnalités, tout ce que vous avez à faire est de changer le XML plutôt que de déblayer et de recompiler du code.Je suis personnellement fan huuuuge :)

Pour plus d'informations, consultez this article pour un bref aperçu des IoC puis consultez this article for the documentation

6

Voici deux variantes en utilisant les énumérations que (presque) fournissent le même comportement d'une manière beaucoup plus lisible:

1) énumérations pour un commutateur de type sécurisé:

enum CommandType { 
MESSAGE, 
QUIT, 
CONFIRMCONNECT 
} 

public void handleParsedCommand(String[] commandArr) { 
    CommandType cmd = null; 

    try { 
     cmd = CommandType.valueOf(commandArr[0].toUpperCase()); 
    } catch (IllegalArgumentException e) { 
     // this kind of error handling, seems a bit strange, by the way. 
     try { 
      out.write("Unknown: " + commandArr[0] + "\n"); 
     } catch (IOException e) { 
      System.out.println("Failed output warning of unknown command."); 
     } 
     return; 
    } 
    switch(cmd) { 
     case MESSAGE: 
      int target = Integer.parseInt(commandArr[1]); 
      String message = commandArr[2]; 
      MachatServer.sendMessage(target, this.conId, message); 
     case QUIT: 
      // Tell the server to disconnect us. 
      MachatServer.disconnect(conId); 
     case CONFIRMCONNECT: 
      // Blah blah and so on for another 10 types of command 
     } 
    } 
} 

les principaux avantages sont que le code est plus lisible, mais vous Avoi d créer de nouvelles méthodes ou classes pour chacun des cas, ce qui ne permet pas ce que vous voulez si le code de gestion n'a qu'une ou deux lignes.

2) Une autre variante basée ENUM, qui est en fait un modèle de commande, mais qui en code beaucoup ballonnement:

enum CommandType { 
    MESSAGE { 
     void execute(CommandProcessor cp, String[] params) { 
      int target = Integer.parseInt(params[1]); 
      String message = params[2]; 
      MachatServer.sendMessage(target, cp.conId, message);   
     } 
    }, 
    QUIT { 
     void execute(CommandProcessor cp, params param) { 
      MachatServer.disconnect(cp.conId); 
     } 
    }, 
    CONFIRMCONNECT { 
     void execute(CommandProcessor cp, params param) { 
       // Blah blah and so on for another 10 types of command 
     } 
    }; 

    abstract void execute(CommandProcessor cp, String[] param); 
} 
public void handleParsedCommand(String[] commandArr) { 
    CommandType cmd = null; 

    try { 
     cmd = CommandType.valueOf(commandArr[0].toUpperCase()); 
    } catch (IllegalArgumentException e) { 
     try { 
      out.write("Unknown: " + commandArr[0] + "\n"); 
     } catch (IOException e) { 
      System.out.println("Failed output warning of unknown command."); 
     } 
     return; 
    } 
    cmd.execute(this, commandArr); 
} 
2

Yeap, ressemble à un modèle Command + Prototype pour moi.

Dans la commande, vous définissez ce qui va être fait, et le prototype est de placer une instance de chaque commande dans une table de recherche et de les "cloner" pour qu'ils soient exécutés à chaque fois.

Le refactoring serait comme:

Avant:

public void handleParsedCommand(String[] commandArr) { 
     if(commandArr[0].equalsIgnoreCase("message")) { 
     int target = Integer.parseInt(commandArr[1]); 
     String message = commandArr[2]; 
     MachatServer.sendMessage(target, this.conId, message); 
    } else if(commandArr[0].equalsIgnoreCase("quit")) { 
     // Tell the server to disconnect us. 
     MachatServer.disconnect(conId); 
    } else if(commandArr[0].equalsIgnoreCase("confirmconnect")) { 
     // Blah blah and so on for another 10 types of command 
    } else { 
     try { 
      out.write("Unknown: " + commandArr[0] + "\n"); 
     } catch (IOException e) { 
      System.out.println("Failed output warning of unknown command."); 
     } 
    } 
} 

Après:

public void handleParsedCommand(String[] commandArr) { 
    Command.getCommand(commandArr).execute(); 
} 


// Define the command and a lookup table 
abstract class Command { 

    // Factory using prototype 
    public static Command getCommand(String [] commandArr) { 
     // find the handling command 
     Command commandPrototype = commandMap.get(commandArr[0]); 
     // if none was found, then use "uknown" 
     if (commandPrototype == null) { 
      commandPrototype = commandMap.get("unknown"); 
     } 
     // Create an instance using clone 
     Command instance = commandPrototype.clone(); 
     instance.args = commanrArr; 
     return instance; 

    } 

    // lookup table (switch substitute) 
    private static Map<String,Command> commandsMap = new HashMap()<String,Command>(){{ 
      put("message"  , new MessagCommand()); 
      put("quit"   , new QuitCommand()); 
      put("confirmconnect", new ConfirmConnectCommand()); 
      ... 
      put("unknow"  , new UnknownCommand()); 

    }}; 


    // args of the command 
    private String [] args; 


    public void execute(); 

    String [] getArgs(){ 
     return this.args; 
    } 


} 

Et fournissent les implémentations spécifiques

class MessageCommand extends Command { 
    public void execute(){ 
     int target = Integer.parseInt(commandArr[1]); 
     String message = commandArr[2]; 
     MachatServer.sendMessage(target, this.conId, message); 
    } 
} 

class MessageCommand extends Command { 
    public void execute(){ 
     int target = Integer.parseInt(getArgs()[1]); 
     String message = getArgs()[2]; 
     MachatServer.sendMessage(target, this.conId, message); 
    } 
} 

class QuitCommand extends Command { 
    public void execute() { 
     MachatServer.disconnect(conId); 
    } 
} 

class ConfirmConnectCommand extends Command { 
    public void execute() { 
    /// blah blah blah 
    } 
} 
class UnknowCommand extends Command { 
    public void execute() { 
     try { 
      out.write("Unknown: " + commandArr[0] + "\n"); 
     } catch (IOException e) { 
      System.out.println("Failed output warning of unknown command."); 
     } 
    } 
} 

// ... other 10 implementations here... 
Questions connexes