2017-12-05 16 views
0

Ich habe diesen Code, der meine Befehle behandelt:Ist es in Ordnung, so viel NEUE Anweisung zu verwenden?

public class CommandHandler { 

    public static final CommandParser parser = new CommandParser(); 
    public static HashMap<String, Command> commands = new HashMap<>(); 

    public static void handleCommand(CommandParser.CommandContainer cmd) { 

     if (commands.containsKey(cmd.invoke)) {   
      commands.get(cmd.invoke).action(cmd.event, cmd.args); 
      commands.get(cmd.invoke).postAction(cmd.event, cmd.args); 
     } 
     /* else 
      System.err.println("CMD NOT EXIST!"); */ 
    } 
} 

public static HashMap<String, Command> commands = new HashMap<>(); 

private static void addCommands() { 
    commands.put(new cmdConfig().getCommand(), new cmdConfig()); 
    commands.put(new cmdHelp().getCommand(), new cmdHelp()); 
    commands.put(new cmdPing().getCommand(), new cmdPing()); 
} 

Dies ist ein kurzer Code, aber ich habe 157 Befehle. Jeder Befehl erstreckt sich von einer Schnittstelle, die diese Methoden haben:

public interface Command { 
    public String getCommand(); 
    public String help(); 
    public void action(String[] args); 
    public void postAction(String[] args); 
} 
Auch

, verwende ich diesen Code zu:

String input = "random"; 

if (new cmdPing().getCommand().equals(input)) { 
    new cmdPing().help(); 
} 
else if (new cmdConfig().getCommand().equals(input)) { 
    new cmdConfig().help(); 
} 
else { 
    printError(input + " is not a valid command"); 
} 

und ich möchte wissen, ob in Ordnung ist, zu viele neue Aussagen zu verwenden, wenn i Verwenden Sie commands.put und innerhalb von if() Aussagen auch.

+0

Was ist sonst in den Befehlsklassen abgesehen von den Strings, die von den beiden Accessor-Methoden zurückgegeben werden, und dem (ich nehme an) gemeinsamen Impl von 'action()'? – Bohemian

+0

Sieht aus, als wäre dein Design kaputt. 'put (new cmdConfig(). getCommand(), new cmdConfig())' sollte mindestens so geschrieben werden wie 'c = new cmdConfig(); put (c.getCommand(), c) '. Sieht so aus, als ob Sie viele Objekte erstellen, nur um eine konstante Zeichenkette zu erhalten, erstellen Sie ein letztes statisches Feld in der Klasse ... –

+0

Danke für die Hilfe! – INeedHelpWithTheCode

Antwort

2

Es ist sehr seltsam, dass Sie eine HashMap<String, Command> commands erstellen, die alle Befehle enthält und dann diese Karte nicht verwendet, um die Befehle nachzuschlagen, was genau der Zweck ist, eine solche Karte zu erstellen.

public static final List<Command> ALL_COMMANDS 
    = Collections.unmodifiableList(Arrays.asList(
     new cmdConfig(), new cmdHelp(), new cmdPing(), … 
    )); 
public static Map<String, Command> ALL_COMMANDS_BY_NAME 
    = ALL_COMMANDS.stream() 
        .collect(Collectors.toMap(Command::getCommand, Function.identity())); 
String input = "random"; 
Command cmd = ALL_COMMANDS_BY_NAME.get(input); 
if(cmd != null) cmd.help(); 
else printError(input + " is not a valid command"); 

Wenn die tatsächlichen Implementierungen der einzelnen Kommandos eher kurz ist, können Sie prüfen Command in einem einzigen, alle von ihnen Umsetzung enum Umsetzung. Dann erhalten Sie die lineare ALL_COMMANDS-Liste kostenlos, aufgrund der generierten values()-Methode, die ein Array aller Konstanten zurückgibt. Der Rest der Anwendungslogik bleibt gleich.

+0

Ich habe vergessen, die CommandHandler-Klasse hinzuzufügen, Sie können es jetzt überprüfen. Ich brauche einen String-Schlüssel in hashmap, so Enum funktioniert nicht für mich. – INeedHelpWithTheCode

+0

Auch ich habe es gerade ausprobiert und es funktioniert perfekt. Es war genau das, was ich brauchte. Wenn Sie einen anderen Fehler in meinem Code feststellen, sagen Sie es mir bitte. Vielen Dank! – INeedHelpWithTheCode

1

Löschen Sie die Schnittstelle und alle implementierenden Klassen.

Stattdessen machen Command ein enum mit final Felder für command und help, Getter für Hilfe und die action() Methode.

Kein Getter für den Befehlsnamen erforderlich, da nur die Klasse selbst verwendet werden muss.

Wenn Sie die enum-Instanz für eine bestimmte Zeichenfolge abrufen müssen, verwenden Sie die valueOf()-Methode, die alle enums haben.

+0

Danke für die Hilfe! – INeedHelpWithTheCode

Verwandte Themen