Existe-t-il un moyen plus intelligent de le faire en plus d'une longue chaîne d'instructions if ou de changement?

20

J'implémente un bot IRC qui reçoit un message et je vérifie ce message pour déterminer les fonctions à appeler. Existe-t-il une manière plus intelligente de procéder? Il semble que cela deviendrait rapidement incontrôlable après que je me sois levé pour aimer 20 commandes.

Peut-être y a-t-il une meilleure façon d'abstraire cela?

 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
la source
14
Quelle langue, son genre d'importance à ce niveau de détail.
mattnz
3
@mattnz toute personne familiarisée avec Java le reconnaîtra dans l'exemple de code qu'il fournit.
jwenting
6
@jwenting: C'est aussi une syntaxe C # valide, et je parie qu'il y a plus de langages.
phresnel
4
@phresnel oui, mais ceux-ci ont-ils exactement la même API standard pour String?
jwenting
3
@jwenting: est-ce pertinent? Mais même si: on peut construire des exemples valides, comme par exemple une bibliothèque d'aide Java / C # -Interop, ou jeter un œil à Java pour .net: ikvm.net. La langue est toujours pertinente. L'interrogateur peut ne pas rechercher de langages spécifiques, il / elle peut avoir commis des erreurs de syntaxe (conversion accidentelle de Java en C #, par exemple), de nouveaux langages peuvent apparaître (ou ont surgi dans la nature, où il y a des dragons) - modifier : Mes commentaires précédents étaient à dicky, désolé.
phresnel

Réponses:

45

Utilisez une table de répartition . Il s'agit d'un tableau contenant des paires ("partie message", pointer-to-function). Le répartiteur ressemblera alors à ceci (en pseudo code):

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

(le equalsIgnoreCasepeut être traité comme un cas spécial quelque part auparavant, ou si vous avez plusieurs de ces tests, avec une deuxième table de répartition).

Bien sûr, ce pointer-to-functionà quoi cela doit ressembler dépend de votre langage de programmation. Voici un exemple en C ou C ++. En Java ou en C #, vous utiliserez probablement des expressions lambda à cet effet, ou vous simulez des "pointeurs vers des fonctions" en utilisant le modèle de commande. Le livre en ligne gratuit " Higher Order Perl " contient un chapitre complet sur les tables de répartition utilisant Perl.

Doc Brown
la source
4
Le problème avec cela, cependant, est que vous ne pourrez pas contrôler le mécanisme de correspondance. Dans l'exemple d'OP, il utilise equalsIgnoreCasepour "jouer maintenant" mais toLowerCase().startsWithpour les autres.
mrjink
5
@mrjink: Je ne vois pas cela comme un "problème", c'est seulement une approche différente avec des avantages et des inconvénients différents. Le «pro» de votre solution: les commandes individuelles peuvent avoir des mécanismes de correspondance individuels. Le "pro" du mien: les commandements n'ont pas à fournir leur propre mécanisme d'appariement. Le PO doit décider quelle solution lui convient le mieux. Au fait, j'ai également voté pour votre réponse.
Doc Brown
1
FYI - "Pointer to function" est une fonctionnalité du langage C # appelée délégués. Lambda est plus un objet d'expression qui peut être transmis - vous ne pouvez pas "appeler" un lambda comme vous pouvez appeler un délégué.
user1068
1
Hissez l' toLowerCaseopération hors de la boucle.
zwol
1
@HarrisonNguyen: Je recommande docs.oracle.com/javase/tutorial/java/javaOO/… . Mais si vous n'utilisez pas Java 8, vous pouvez simplement utiliser la définition d'interface de mrink sans la partie "matches", c'est 100% équivalent (c'est ce que je voulais dire par "simuler le pointeur vers la fonction en utilisant le modèle de commande). Et user1068's commentaire est juste une remarque sur les différents termes pour différentes variantes du même concept dans différents langages de programmation
Doc Brown
31

Je ferais probablement quelque chose comme ça:

public interface Command {
  boolean matches(String message);

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

Ensuite, vous pouvez demander à chaque commande d'implémenter cette interface et de renvoyer true lorsqu'elle correspond au message.

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
la source
Si les messages n'ont jamais besoin d'être analysés ailleurs (j'ai supposé que dans ma réponse) c'est en effet préférable à ma solution car elle Commandest plus autonome si elle sait quand être appelée elle-même. Cela produit une légère surcharge si la liste des commandes est énorme mais c'est probablement négligeable.
2014
1
Ce modèle a tendance à bien s'adapter au paradygme des commandes de la console, mais uniquement parce qu'il sépare soigneusement la logique de commande du bus d'événements et parce que de nouvelles commandes sont susceptibles d'être ajoutées à l'avenir. Je pense qu'il convient de noter que ce n'est pas une solution dans toutes les circonstances où vous avez une longue chaîne de si ... elseif
Neil
+1 pour l'utilisation d'un modèle de commande "léger"! Il convient de noter que lorsque vous traitez avec des non-chaînes, vous pouvez utiliser, par exemple, des énumérations intelligentes qui savent comment exécuter leur logique. Cela vous permet même d'économiser la boucle for.
LastFreeNickname
3
Plutôt que la boucle, nous pourrions l'utiliser comme une carte si vous faites de Command une classe abstraite qui remplace equalset hashCodepour être la même que la chaîne qui représente la commande
Cruncher
C'est génial et facile à comprendre. Merci pour la suggestion. C'est à peu près exactement ce que je cherchais et semble être gérable pour l'ajout futur de commandes.
Harrison Nguyen
15

Vous utilisez Java - alors rendez-le beau ;-)

Je le ferais probablement en utilisant des annotations:

  1. Créer une annotation de méthode personnalisée

    @IRCCommand( String command, boolean perfectmatch = false )
  2. Ajoutez l'annotation à toutes les méthodes pertinentes de la classe, par exemple

    @IRCCommand( command = ".np", perfectmatch = true )
    doNP( ... )
  3. Dans votre constructeur, utilisez Reflections pour créer un HashMap de méthodes à partir de toutes les méthodes annotées de votre classe:

    ...
    for (Method m : getDeclaredMethods()) {
    if ( isAnnotationPresent... ) {
        commandList.put(m.getAnnotation(...), m);
        ...
  4. Dans votre onMessageméthode, il suffit de faire une boucle pour commandListessayer de faire correspondre la chaîne sur chacun et d'appeler method.invoke()où elle se situe.

    for ( @IRCCommand a : commanMap.keyList() ) {
        if ( cmd.equalsIgnoreCase( a.command )
             || ( cmd.startsWith( a.command ) && !a.perfectMatch ) {
            commandMap.get( a ).invoke( this, cmd );
Falco
la source
Il n'est pas clair qu'il utilise Java, bien que ce soit une solution élégante.
Neil
Vous avez raison - le code ressemblait tellement au code Java au format auto éclipsé ... Mais vous pourriez faire la même chose avec C # et avec C ++ vous pourriez simuler des annotations avec des macros intelligentes
Falco
Cela pourrait très bien être Java, mais à l'avenir, je vous suggère d'éviter les solutions spécifiques au langage si le langage n'est pas clairement indiqué. Juste des conseils amicaux.
Neil
Merci pour cette solution - vous aviez raison d'utiliser Java dans le code fourni même si je ne l'ai pas spécifié, cela a l'air vraiment sympa et je vais être sûr de lui donner un coup de feu. Désolé, je ne peux pas choisir deux meilleures réponses!
Harrison Nguyen
5

Que se passe-t-il si vous définissez une interface, disons IChatBehaviourlaquelle a une méthode appelée Executequi prend un messageet un cmdobjet:

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

Dans votre code, vous implémentez ensuite cette interface et définissez les comportements que vous souhaitez:

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

Et ainsi de suite pour le reste.

Dans votre classe principale, vous avez alors une liste de comportements ( List<IChatBehaviour>) que votre bot IRC implémente. Vous pouvez ensuite remplacer vos ifdéclarations par quelque chose comme ceci:

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

Ce qui précède devrait réduire la quantité de code dont vous disposez. L'approche ci-dessus vous permettrait également de fournir des comportements supplémentaires à votre classe de bot sans modifier la classe de bot elle-même (selon le Strategy Design Pattern).

Si vous souhaitez qu'un seul comportement se déclenche à la fois, vous pouvez modifier la signature de la executeméthode pour produire true(le comportement s'est déclenché) ou false(le comportement ne s'est pas déclenché) et remplacer la boucle ci-dessus par quelque chose comme ceci:

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

Ce qui précède serait plus fastidieux à mettre en œuvre et à initialiser car vous devez créer et passer toutes les classes supplémentaires, cependant, cela devrait rendre votre bot facilement extensible et modifiable car toutes vos classes de comportement seront encapsulées et, espérons-le, indépendantes les unes des autres.

npinti
la source
1
Où sont ifpassés les s? C'est-à-dire, comment décidez-vous qu'un comportement est exécuté pour une commande?
mrjink
2
@mrjink: Désolé mon mauvais. La décision d'exécuter ou non est déléguée au comportement (j'ai omis par erreur la ifpartie du comportement).
npinti
Je pense que je préfère vérifier si une donnée IChatBehaviourpeut gérer une commande donnée, car elle permet à l'appelant d'en faire plus, comme traiter les erreurs si aucune commande ne correspond, bien que ce ne soit vraiment qu'une préférence personnelle. Si cela n'est pas nécessaire, inutile de compliquer inutilement le code.
Neil
vous auriez toujours besoin d'un moyen de générer l'instance de la classe correcte afin de l'exécuter, qui aurait toujours la même longue chaîne d'if ou une instruction switch massive ...
jwenting
1

"Intelligent" peut être (au moins) trois choses:

Des performances supérieures

La suggestion du tableau de répartition (et de ses équivalents) est bonne. Une telle table s'appelait "CADET" dans les années passées pour "Impossible d'ajouter; N'essaye même pas." Cependant, considérez un commentaire pour aider un mainteneur novice à savoir comment gérer ladite table.

Maintenabilité

"Make it beautiful" n'est pas un avertissement inutile.

et, souvent négligé ...

Élasticité

L'utilisation de toLowerCase présente des pièges dans la mesure où certains textes dans certaines langues doivent subir une restructuration douloureuse lors du passage entre magiscule et minuscule. Malheureusement, les mêmes pièges existent pour toUpperCase. Soyez juste conscient.

We B Martians
la source
0

Vous pourriez avoir toutes les commandes implémenter la même interface. Un analyseur de messages pourrait alors vous renvoyer la commande appropriée que vous n'exécuterez que.

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

Il ressemble à juste plus de code. Oui, vous devez toujours analyser le message afin de savoir quelle commande exécuter, mais maintenant elle est à un point correctement défini. Il peut être réutilisé ailleurs. (Vous voudrez peut-être injecter le MessageParser mais c'est une autre question. En outre, le modèle Flyweight peut être une bonne idée pour les commandes, selon le nombre que vous prévoyez d'être créé.)

jhr
la source
Je pense que c'est bon pour le découplage et l'organisation du programme, bien que je ne pense pas que cela résout directement le problème d'avoir trop de déclarations if ... elseif.
Neil
0

Ce que je ferais c'est ceci:

  1. Regroupez les commandes que vous avez en groupes. (vous en avez au moins 20 en ce moment)
  2. Au premier niveau, catégorisez par groupe, de sorte que vous ayez une commande liée au nom d'utilisateur, des commandes de morceau, des commandes de comptage, etc.
  3. Ensuite, vous allez dans la méthode de chaque groupe, cette fois, vous obtiendrez la commande d'origine.

Cela rendra cela plus gérable. Plus d'avantages lorsque le nombre de «sinon si» augmente trop.

Bien sûr, avoir parfois ces «sinon» ne serait pas un gros problème. Je ne pense pas que 20 est si mauvais.

InforméA
la source