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

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);
        }
    }

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.

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
    }
}

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 );

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.

"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.

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.)

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.