Czy istnieje bardziej inteligentny sposób na zrobienie tego oprócz długiego łańcucha instrukcji if lub switch?

20

Wdrażam bota IRC, który odbiera wiadomość i sprawdzam tę wiadomość, aby określić, które funkcje wywołać. Czy jest na to bardziej sprytny sposób? Wygląda na to, że szybko wymknął się spod kontroli po tym, jak wykonałem 20 poleceń.

Być może istnieje lepszy sposób na streszczenie tego?

 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
źródło
14
Jaki język, jego rodzaj jest ważny na tym poziomie szczegółowości.
mattnz
3
@mattnz każdy, kto zna Javę, rozpozna ją w dostarczonym przez siebie przykładzie kodu.
jwenting
6
@jwenting: Jest to również poprawna składnia C # i założę się, że jest więcej języków.
fresnel
4
@ phresnel tak, ale czy te mają dokładnie taki sam standardowy interfejs API dla String?
jwenting
3
@jwenting: Czy to istotne? Ale nawet jeśli: Można skonstruować prawidłowe przykłady, takie jak na przykład Java / C # -Interop Helper Library, lub spojrzeć na Javę dla .net: ikvm.net. Język jest zawsze odpowiedni. Pytający może nie szukać określonych języków, mógł popełnić błędy składniowe (przypadkowo przekonwertować Javę na C #, np.), Mogą pojawić się nowe języki (lub wyrosły na wolności, gdzie są smoki) - edytuj : Moje poprzednie komentarze były dicky, przepraszam.
fresnel

Odpowiedzi:

45

Użyj tabeli wysyłki . To jest tabela zawierająca pary („część wiadomości”, pointer-to-function). Dyspozytor będzie więc wyglądał następująco (w pseudo-kodzie):

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

( equalsIgnoreCasemoże być traktowany gdzieś wcześniej jako specjalny przypadek lub jeśli masz wiele z tych testów, z drugą tabelą wysyłkową).

Oczywiście to, jak pointer-to-functionmusi wyglądać, zależy od języka programowania. Oto przykład w C lub C ++. W Javie lub C # prawdopodobnie użyjesz do tego celu wyrażeń lambda lub symulujesz „wskaźnik do funkcji” za pomocą wzorca poleceń. Bezpłatna książka online „ Wyższy porządek Perl ” zawiera pełny rozdział na temat tabel wysyłkowych przy użyciu Perla.

Doktor Brown
źródło
4
Problem polega na tym, że nie będziesz w stanie kontrolować mechanizmu dopasowywania. W przykładzie OP używa equalsIgnoreCase„teraz gra”, ale toLowerCase().startsWithdla innych.
mrjink
5
@mrjink: Nie uważam tego za „problem”, jest to tylko inne podejście z różnymi zaletami i wadami. Zaleta rozwiązania: poszczególne polecenia mogą mieć indywidualne mechanizmy dopasowywania. Mój „pro”: komendy nie muszą zapewniać własnego mechanizmu dopasowywania. OP musi zdecydować, które rozwiązanie mu najbardziej odpowiada. Nawiasem mówiąc, głosowałem również za odpowiedzią.
Doc Brown
1
FYI - „Wskaźnik do funkcji” to funkcja języka C # zwana delegatami. Lambda jest bardziej obiektem wyrażenia, który można przekazywać - nie można „wywołać” lambda, tak jak można nazwać delegata.
user1068
1
Dźwignicy toLowerCasedziałanie na pętlę.
zwolnienie
1
@HarrisonNguyen: Polecam docs.oracle.com/javase/tutorial/java/javaOO/… . Ale jeśli nie używasz Java 8, możesz po prostu użyć definicji interfejsu mrink bez części „dopasowania”, to jest w 100% równoważne (to miałem na myśli przez „symulację wskaźnika do funkcji za pomocą wzorca poleceń). komentarz to tylko uwaga na temat różnych terminów dla różnych wariantów tej samej koncepcji w różnych językach programowania
Doc Brown
31

Prawdopodobnie zrobiłbym coś takiego:

public interface Command {
  boolean matches(String message);

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

Następnie możesz sprawić, by każde polecenie zaimplementowało ten interfejs i zwróciło true, gdy pasuje do komunikatu.

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
źródło
Jeśli wiadomości nigdy nie muszą być analizowane gdzie indziej (założyłem, że w mojej odpowiedzi) jest to rzeczywiście lepsze niż moje rozwiązanie, ponieważ Commandjest bardziej samowystarczalne, jeśli wie, kiedy należy się nazywać. Powoduje to niewielki narzut, jeśli lista poleceń jest ogromna, ale to prawdopodobnie nieistotne.
jhr
1
Ten wzorzec ładnie pasuje do paradygmatu poleceń konsoli, ale tylko dlatego, że starannie oddziela logikę poleceń od magistrali zdarzeń i ponieważ prawdopodobnie nowe polecenia zostaną dodane w przyszłości. Myślę, że należy zauważyć, że nie jest to rozwiązanie na każdą okoliczność, w której masz długi łańcuch if ... elseif
Neil
+1 za użycie „lekkiego” schematu poleceń! Należy zauważyć, że kiedy masz do czynienia z nie-Ciągami, możesz skorzystać np. Z inteligentnych wyliczeń, które wiedzą, jak wykonywać swoją logikę. To nawet oszczędza ci pętli for.
LastFreeNickname
3
Zamiast pętli moglibyśmy użyć tego jako mapy, jeśli uczynisz Command abstrakcyjną klasą, która przesłania equalsi hashCodejest taka sama jak ciąg znaków reprezentujący polecenie
Cruncher
To jest niesamowite i łatwe do zrozumienia. Dzieki za sugestie. Jest to dokładnie to, czego szukałem i wydaje się, że da się zarządzać przy dodawaniu poleceń w przyszłości.
Harrison Nguyen,
15

Używasz Javy - więc upiększ ją ;-)

Prawdopodobnie zrobiłbym to za pomocą Adnotacji:

  1. Utwórz niestandardową adnotację metody

    @IRCCommand( String command, boolean perfectmatch = false )
  2. Dodaj adnotację do wszystkich odpowiednich metod w klasie, np

    @IRCCommand( command = ".np", perfectmatch = true )
    doNP( ... )
  3. W swoim konstruktorze użyj Reflections, aby utworzyć HashMap metod ze wszystkich metod z adnotacjami w twojej klasie:

    ...
    for (Method m : getDeclaredMethods()) {
    if ( isAnnotationPresent... ) {
        commandList.put(m.getAnnotation(...), m);
        ...
  4. W swojej onMessagemetodzie po prostu wykonaj pętlę, commandListpróbując dopasować ciąg na każdym z nich i wywołać method.invoke()tam, gdzie pasuje.

    for ( @IRCCommand a : commanMap.keyList() ) {
        if ( cmd.equalsIgnoreCase( a.command )
             || ( cmd.startsWith( a.command ) && !a.perfectMatch ) {
            commandMap.get( a ).invoke( this, cmd );
Falco
źródło
Nie jest jasne, czy używa Javy, ale jest to eleganckie rozwiązanie.
Neil
Masz rację - kod wyglądał tak bardzo podobnie do automatycznie kodowanego kodu Java Eclipse ... Ale możesz zrobić to samo z C #, a z C ++ możesz symulować adnotacje za pomocą sprytnych makr
Falco
Równie dobrze mogłaby to być Java, jednak w przyszłości sugeruję unikanie rozwiązań specyficznych dla języka, jeśli język nie jest wyraźnie wskazany. Po prostu przyjacielska rada.
Neil
Dzięki za to rozwiązanie - miałeś rację, że używam Javy w dostarczonym kodzie, chociaż nie określiłem tego, to wygląda naprawdę ładnie i na pewno dam mu szansę. Niestety nie mogę wybrać dwóch najlepszych odpowiedzi!
Harrison Nguyen,
5

Co się stanie, jeśli zdefiniujesz interfejs, powiedzmy, IChatBehaviourktóra ma wywoływaną jedną metodę, Executektóra przyjmuje a messagei cmdobiekt:

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

W swoim kodzie implementujesz ten interfejs i definiujesz pożądane zachowania:

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

I tak dalej.

W swojej głównej klasie masz listę zachowań ( List<IChatBehaviour>), które implementuje twój bot IRC. Następnie możesz zamienić swoje ifwypowiedzi na takie:

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

Powyższe powinno zmniejszyć ilość posiadanego kodu. Powyższe podejście pozwoliłoby również na dostarczenie dodatkowych zachowań do twojej klasy bota bez modyfikowania samej klasy bota (zgodnie z Strategy Design Pattern).

Jeśli chcesz, aby zadziałało tylko jedno zachowanie w danym momencie, możesz zmienić sygnaturę executemetody, aby uzyskać true(zachowanie zadziałało) lub false(zachowanie nie zadziałało) i zastąpić powyższą pętlę czymś takim:

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

Powyższe byłoby bardziej żmudne we wdrażaniu i inicjowaniu, ponieważ musisz utworzyć i przekazać wszystkie dodatkowe klasy, jednak powinno to uczynić twojego bota łatwym do rozszerzenia i modyfikowania, ponieważ wszystkie twoje klasy zachowań będą zamknięte i mam nadzieję, że będą od siebie niezależne.

npinti
źródło
1
Gdzie ifposzło? Tj. Jak zdecydujesz, że zachowanie jest wykonywane dla polecenia?
mrjink
2
@mrjink: Przepraszam mój zły. Decyzja o tym, czy wykonać, czy nie, zależy od zachowania (omyłkowo pominąłem ifczęść tego zachowania).
npinti
Wydaje mi się, że wolę sprawdzenie, czy dana IChatBehaviourporadzi sobie z danym poleceniem, ponieważ pozwala ono wywołującemu zrobić z nim więcej, na przykład przetwarzać błędy, jeśli żadne polecenie nie pasuje, chociaż tak naprawdę jest to tylko osobiste preferencje. Jeśli to nie jest potrzebne, nie ma sensu niepotrzebnie komplikować kodu.
Neil
nadal potrzebujesz sposobu na wygenerowanie instancji poprawnej klasy w celu jej wykonania, która nadal miałaby ten sam długi łańcuch instrukcji ifs lub masywną instrukcję przełączania ...
jwenting
1

„Inteligentny” może obejmować (przynajmniej) trzy rzeczy:

Wyższa wydajność

Sugestia tabeli wysyłki (i jej odpowiedników) jest dobra. Taki stół nazywał się w przeszłości „CADET” dla „Nie można dodać, nawet nie próbuje”. Zastanów się jednak nad komentarzem, który pomoże nowemu opiekunowi, jak zarządzać wspomnianą tabelą.

Konserwowalność

„Make it beautiful” nie jest bezczynnym napomnieniem.

i często pomijane ...

Odporność

Korzystanie z funkcji toLowerCase wiąże się z pułapkami polegającymi na tym, że niektóre teksty w niektórych językach muszą podlegać bolesnej restrukturyzacji przy zmianie między magiscule a maleinule. Niestety w przypadku toUpperCase istnieją takie same pułapki. Po prostu bądź świadomy.

My B Marsjanie
źródło
0

Możesz mieć wszystkie polecenia implementujące ten sam interfejs. Następnie analizator komunikatów może zwrócić odpowiednie polecenie, które wykonasz tylko.

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

Wygląda jak tylko więcej kodu. Tak, nadal musisz przeanalizować komunikat, aby wiedzieć, które polecenie wykonać, ale teraz jest ono we właściwie zdefiniowanym punkcie. Może być ponownie użyty w innym miejscu. (Być może chcesz wstrzyknąć MessageParser, ale to inna sprawa. Ponadto wzorzec Flyweight może być dobrym pomysłem dla poleceń, w zależności od tego, ile utworzysz.)

jhr
źródło
Myślę, że jest to dobre dla oddzielenia i organizacji programu, ale nie sądzę, że to bezpośrednio rozwiązuje problem z zbyt dużą liczbą instrukcji if ... elseif.
Neil
0

Chciałbym to zrobić:

  1. Pogrupuj posiadane polecenia w grupy. (masz teraz co najmniej 20)
  2. Na pierwszym poziomie, kategoryzuj według grup, więc masz komendy związane z nazwą użytkownika, komendy piosenki, komendy liczenia itp.
  3. Następnie zastosuj metodę każdej grupy, tym razem otrzymasz oryginalne polecenie.

Ułatwi to zarządzanie. Więcej korzyści, gdy liczba „else if” zbytnio rośnie.

Oczywiście czasami posiadanie tych „jeśli inaczej” nie byłoby dużym problemem. Nie sądzę, że 20 jest tak źle.

Poinformowano
źródło