Gibt es eine intelligentere Möglichkeit, dies zu tun, als nur eine lange Kette von if-Anweisungen oder -Schaltern?

20

Ich implementiere einen IRC-Bot, der eine Nachricht empfängt, und überprüfe diese Nachricht, um festzustellen, welche Funktionen aufgerufen werden sollen. Gibt es eine klügere Art, dies zu tun? Es scheint, als würde es schnell außer Kontrolle geraten, nachdem ich 20 Befehle erhalten habe.

Vielleicht gibt es einen besseren Weg, dies zu abstrahieren?

 public void onMessage(String channel, String sender, String login, String hostname, String message){

        if (message.equalsIgnoreCase(".np")){
//            TODO: Use Last.fm API to find the now playing
        } else if (message.toLowerCase().startsWith(".register")) {
                cmd.registerLastNick(channel, sender, message);
        } else if (message.toLowerCase().startsWith("give us a countdown")) {
                cmd.countdown(channel, message);
        } else if (message.toLowerCase().startsWith("remember am routine")) {
                cmd.updateAmRoutine(channel, message, sender);
        }
    }
Harrison Nguyen
quelle
14
Welche Sprache ist auf dieser Detailebene wichtig?
Mattnz
3
@mattnz Jeder, der mit Java vertraut ist, wird es in dem von ihm bereitgestellten Codebeispiel erkennen.
Jwenting
6
@jwenting: Es ist auch eine gültige C # -Syntax und ich wette, es gibt mehr Sprachen.
Phresnel
4
@phresnel ja, aber haben diese genau die gleiche Standard-API für String?
Jwenting
3
@jwenting: Ist das relevant? Aber selbst wenn: Man kann gültige Beispiele erstellen, wie zum Beispiel eine Java / C # -Interop-Hilfsbibliothek, oder sich Java für .net: ikvm.net ansehen. Die Sprache ist immer relevant. Der Fragesteller sucht möglicherweise nicht nach bestimmten Sprachen, er / sie hat möglicherweise Syntaxfehler begangen (z. B. durch versehentliches Konvertieren von Java in C #), es können neue Sprachen aufgetaucht sein (oder er / sie ist in freier Wildbahn aufgetaucht, wo es Drachen gibt) - Bearbeiten : Meine vorherigen Kommentare waren zu dicky, sorry.
Phresnel

Antworten:

45

Verwenden Sie eine Versandtabelle . Dies ist eine Tabelle, die Paare enthält ("Nachrichtenteil", pointer-to-function). Der Dispatcher sieht dann so aus (in Pseudocode):

for each (row in dispatchTable)
{
    if(message.toLowerCase().startsWith(row.messagePart))
    {
         row.theFunction(message);
         break;
    }
}

( equalsIgnoreCaseDies kann als Sonderfall behandelt werden, oder wenn Sie viele dieser Tests haben, mit einer zweiten Versandtabelle).

Wie pointer-to-functiondas aussehen muss, hängt natürlich von Ihrer Programmiersprache ab. Hier ist ein Beispiel in C oder C ++. In Java oder C # werden Sie wahrscheinlich Lambda-Ausdrücke für diesen Zweck verwenden, oder Sie simulieren "Zeiger auf Funktionen" mithilfe des Befehlsmusters. Das kostenlose Online-Buch " Higher Order Perl " enthält ein vollständiges Kapitel über Versandtabellen mit Perl.

Doc Brown
quelle
4
Das Problem dabei ist jedoch, dass Sie den Abgleichmechanismus nicht steuern können. In OPs Beispiel verwendet equalsIgnoreCaseer "Jetzt spielen", aber toLowerCase().startsWithfür die anderen.
Mrjink
5
@mrjink: Ich sehe das nicht als "Problem", es ist nur ein anderer Ansatz mit unterschiedlichen Vor- und Nachteilen. Das "Pro" Ihrer Lösung: Individuelle Befehle können individuelle Matching-Mechanismen haben. Das "Pro" von mir: Die Befehle müssen keinen eigenen Matching-Mechanismus bereitstellen. Das OP muss entscheiden, welche Lösung am besten zu ihm passt. Übrigens habe ich auch Ihre Antwort positiv bewertet.
Doc Brown
1
Zu Ihrer Information - "Pointer to Function" ist eine C # -Sprachenfunktion, die als "Delegates" bezeichnet wird. Lambda ist eher ein Ausdrucksobjekt, das weitergegeben werden kann - Sie können ein Lambda nicht so "nennen", wie Sie einen Delegierten anrufen können.
user1068
1
Heben Sie den toLowerCaseBetrieb aus der Schleife.
zwol
1
@ HarrisonNguyen: Ich empfehle docs.oracle.com/javase/tutorial/java/javaOO/… . Wenn Sie jedoch nicht Java 8 verwenden, können Sie die Schnittstellendefinition von mrink auch ohne den Teil "Übereinstimmungen" verwenden, der zu 100% äquivalent ist (das habe ich mit "Zeiger auf Funktion mithilfe des Befehlsmusters simulieren" gemeint) Kommentar ist nur eine Bemerkung zu den verschiedenen Begriffen für verschiedene Varianten des gleichen Konzepts in verschiedenen Programmiersprachen.
Doc Brown
31

Ich würde wahrscheinlich so etwas machen:

public interface Command {
  boolean matches(String message);

  void execute(String channel, String sender, String login,
               String hostname, String message);
}

Dann kann jeder Befehl diese Schnittstelle implementieren und true zurückgeben, wenn sie mit der Nachricht übereinstimmt.

List<Command> activeCommands = new ArrayList<>();
activeCommands.add(new LastFMCommand());
activeCommands.add(new RegisterLastNickCommand());
// etc.

for (Command command : activeCommands) {
    if (command.matches(message)) {
        command.execute(channel, sender, login, hostname, message);
        break; // handle the first matching command only
    }
}
mrjink
quelle
Wenn Nachrichten nie anderswo analysiert werden müssen (ich nahm an, dass dies in meiner Antwort der Fall war), ist dies meiner Lösung in der Tat vorzuziehen, Commandda sie in sich geschlossener ist, wenn sie weiß, wann sie selbst aufgerufen werden muss. Wenn die Liste der Befehle sehr umfangreich ist, entsteht ein geringer Overhead, der jedoch wahrscheinlich vernachlässigbar ist.
13.
1
Dieses Muster passt in der Regel gut zum Konsolenbefehlsparadygma, aber nur, weil es die Befehlslogik sauber vom Ereignisbus trennt und weil in Zukunft wahrscheinlich neue Befehle hinzugefügt werden. Ich denke, es sollte beachtet werden, dass dies keine Lösung für alle Umstände ist, in denen Sie eine lange Kette von if ... elseif
Neil
+1 für die Verwendung eines "leichten" Befehlsmusters! Es sollte beachtet werden, dass Sie, wenn Sie sich mit Nicht-Strings befassen, beispielsweise intelligente Enums verwenden können, die wissen, wie ihre Logik ausgeführt wird. Dies erspart Ihnen sogar die for-Schleife.
LastFreeNickname
3
Anstelle der Schleife können Sie dies als Map verwenden, wenn Sie Command zu einer abstrakten Klasse machen, die überschreibt equalsund hashCodeder Zeichenfolge entspricht, die den Befehl darstellt
Cruncher
Das ist großartig und leicht zu verstehen. Danke für den Vorschlag. Es ist ziemlich genau das, wonach ich gesucht habe und scheint für die zukünftige Hinzufügung von Befehlen handhabbar zu sein.
Harrison Nguyen
15

Du benutzt Java - also mach es schön ;-)

Ich würde dies wahrscheinlich mit Anmerkungen tun:

  1. Erstellen Sie eine benutzerdefinierte Methodenanmerkung

    @IRCCommand( String command, boolean perfectmatch = false )
  2. Fügen Sie die Annotation zu allen relevanten Methoden in der Klasse hinzu, z

    @IRCCommand( command = ".np", perfectmatch = true )
    doNP( ... )
  3. Verwenden Sie in Ihrem Konstruktor Reflections, um eine HashMap von Methoden aus allen mit Annotationen versehenen Methoden in Ihrer Klasse zu erstellen:

    ...
    for (Method m : getDeclaredMethods()) {
    if ( isAnnotationPresent... ) {
        commandList.put(m.getAnnotation(...), m);
        ...
  4. Machen Sie in Ihrer onMessageMethode einfach eine Schleife, commandListindem Sie versuchen, den String für jeden einzelnen zuzuordnen, und rufen method.invoke()Sie auf , wo er passt.

    for ( @IRCCommand a : commanMap.keyList() ) {
        if ( cmd.equalsIgnoreCase( a.command )
             || ( cmd.startsWith( a.command ) && !a.perfectMatch ) {
            commandMap.get( a ).invoke( this, cmd );
Falco
quelle
Es ist nicht klar, dass er Java verwendet, obwohl das eine elegante Lösung ist.
Neil
Sie haben Recht - der Code sah nur so ähnlich aus wie Eclipse-automatisch formatierter Java-Code ... Aber Sie könnten dasselbe mit C # tun und mit C ++ könnten Sie Anmerkungen mit einigen cleveren Makros simulieren
Falco
Es könnte sehr gut Java sein, aber ich schlage vor, dass Sie in Zukunft sprachspezifische Lösungen vermeiden, wenn die Sprache nicht eindeutig angegeben wird. Einfach freundlicher Rat.
Neil
Vielen Dank für diese Lösung - Sie hatten Recht, dass ich Java in dem bereitgestellten Code verwende, obwohl ich es nicht angegeben habe. Das sieht wirklich gut aus und ich werde es auf jeden Fall ausprobieren. Tut mir leid, ich kann nicht zwei beste Antworten auswählen!
Harrison Nguyen
5

Was ist, wenn Sie eine Schnittstelle definieren, z. B. IChatBehaviourwelche Methode aufgerufen wird und ein Objekt Executeaufnimmt :messagecmd

public Interface IChatBehaviour
{
    public void execute(String message, CMD cmd);
}

In Ihrem Code implementieren Sie dann diese Schnittstelle und definieren das gewünschte Verhalten:

public class RegisterLastNick implements IChatBehaviour
{
    public void execute(String message, CMD cmd)
    {
        if (message.toLowerCase().startsWith(".register"))
        {
            cmd.registerLastNick(channel, sender, message);
        }
    }
}

Und so weiter für den Rest.

In Ihrer Hauptklasse haben Sie dann eine Liste von Verhaltensweisen ( List<IChatBehaviour>), die Ihr IRC-Bot implementiert. Sie könnten dann Ihre ifAussagen durch so etwas ersetzen :

for(IChatBehaviour behaviour : this.behaviours)
{
    behaviour.execute(message, cmd);
}

Das Obige sollte die Menge des Codes reduzieren, den Sie haben. Der obige Ansatz würde es Ihnen auch ermöglichen, Ihrer Bot-Klasse zusätzliches Verhalten zuzuweisen, ohne die Bot-Klasse selbst (gemäß der Strategy Design Pattern) zu ändern .

Wenn Sie möchten, dass jeweils nur ein Verhalten ausgelöst wird , können Sie die Signatur der executeMethode ändern , um zu ergeben true(das Verhalten wurde ausgelöst) oder false(das Verhalten wurde nicht ausgelöst ), und die obige Schleife durch Folgendes ersetzen:

for(IChatBehaviour behaviour : this.behaviours)
{
    if(behaviour.execute(message, cmd))
    { 
         break;
    }
}

Die Implementierung und Initialisierung der oben genannten Aktionen ist umso mühsamer, da Sie alle zusätzlichen Klassen erstellen und übergeben müssen. Sie sollten Ihren Bot jedoch leicht erweiterbar und modifizierbar machen, da alle Ihre Verhaltensklassen gekapselt und hoffentlich unabhängig voneinander sind.

npinti
quelle
1
Wohin sind die ifs gegangen? Dh, wie entscheidest du, dass ein Verhalten für einen Befehl ausgeführt wird?
Mrjink
2
@ Mrjink: Entschuldigung, mein Schlechtes. Die Entscheidung, ob ausgeführt werden soll oder nicht, wird an das Verhalten delegiert (ich habe den ifTeil des Verhaltens fälschlicherweise weggelassen ).
Npinti
Ich glaube, ich bevorzuge die Prüfung, ob ein bestimmter IChatBehaviourBefehl einen bestimmten Befehl verarbeiten kann, da der Aufrufer damit mehr anfangen kann, beispielsweise Fehler verarbeiten kann, wenn kein Befehl übereinstimmt, obwohl dies eigentlich nur eine persönliche Präferenz ist. Wenn das nicht benötigt wird, macht es keinen Sinn, Code unnötig zu komplizieren.
Neil
Sie brauchen immer noch eine Möglichkeit, die Instanz der richtigen Klasse zu generieren, um sie auszuführen, die immer noch dieselbe lange Kette von ifs oder massiven switch-Anweisungen hat ...
am
1

"Intelligent" kann (mindestens) drei Dinge sein:

Höhere Leistung

Der Vorschlag für die Versandtabelle (und ihre Entsprechungen) ist gut. Ein solcher Tisch hieß in den vergangenen Jahren "CADET" für "Can't Add; Doesn't Even Try". Überlegen Sie sich jedoch einen Kommentar, um einem unerfahrenen Betreuer die Verwaltung dieser Tabelle zu erläutern.

Wartbarkeit

"Make it beautiful" ist keine müßige Ermahnung.

und oft übersehen ...

Elastizität

Die Verwendung von toLowerCase birgt die Gefahr, dass Text in einigen Sprachen beim Wechsel zwischen Magiscule und Miniscule schmerzhaft umstrukturiert werden muss. Leider gibt es für toUpperCase die gleichen Fallstricke. Sei dir nur bewusst.

Wir B Marsmenschen
quelle
0

Sie könnten alle Befehle dieselbe Schnittstelle implementieren lassen. Dann könnte ein Nachrichtenparser den entsprechenden Befehl zurückgeben, den Sie nur ausführen werden.

public interface Command {
    public void execute(String channel, String message, String sender) throws Exception;
}

public class MessageParser {
    public Command parseCommandFromMessage(String message) {
        // TODO Put your if/switch or something more clever here
        // e.g. return new CountdownCommand();
    }
}

public class Whatever {
    public void onMessage(String channel, String sender, String login, String hostname, String message) {
        Command c = new MessageParser().parseCommandFromMessage(message);
        c.execute(channel, message, sender);
    }
}

Es sieht aus wie nur mehr Code. Ja, Sie müssen die Nachricht noch analysieren, um zu wissen, welcher Befehl ausgeführt werden soll. Jetzt befindet sie sich an einem genau definierten Punkt. Es kann an anderer Stelle wiederverwendet werden. (Möglicherweise möchten Sie den MessageParser einschleusen, aber das ist eine andere Sache. Je nachdem, wie viele Befehle erstellt werden sollen, ist das Flyweight-Muster möglicherweise eine gute Idee für die Befehle.)

jhr
quelle
Ich denke, dies ist gut für die Entkopplung und Programmorganisation, obwohl ich nicht der Meinung bin, dass dies das Problem mit zu vielen if ... elseif-Anweisungen direkt angeht.
Neil
0

Was ich tun würde, ist Folgendes:

  1. Gruppieren Sie die Befehle, die Sie haben, in Gruppen. (Sie haben mindestens 20 im Moment)
  2. In der ersten Ebene kategorisieren Sie nach Gruppen, damit Sie benutzernamenbezogene Befehle, Liedbefehle, Zählbefehle usw. haben.
  3. Dann gehen Sie in die Methode jeder Gruppe, diesmal erhalten Sie den ursprünglichen Befehl.

Dies macht dies leichter handhabbar. Mehr Nutzen, wenn die Anzahl der 'else if' zu stark ansteigt.

Natürlich wäre es manchmal kein großes Problem, diese "wenn sonst" zu haben. Ich denke nicht, dass 20 so schlimm ist.

InformedA
quelle