Unikanie voodoo `goto`?

Mam switchstrukturę, która ma kilka spraw do załatwienia. switchPracuje przy enumco stwarza problem powielonych kodu poprzez wspólne wartości:

// All possible combinations of One - Eight.
public enum ExampleEnum {
    One,
    Two, TwoOne,
    Three, ThreeOne, ThreeTwo, ThreeOneTwo,
    Four, FourOne, FourTwo, FourThree, FourOneTwo, FourOneThree,
          FourTwoThree, FourOneTwoThree
    // ETC.
}

Obecnie switchstruktura obsługuje każdą wartość osobno:

// All possible combinations of One - Eight.
switch (enumValue) {
    case One: DrawOne; break;
    case Two: DrawTwo; break;
    case TwoOne:
        DrawOne;
        DrawTwo;
        break;
     case Three: DrawThree; break;
     ...
}

Masz pomysł. Obecnie mam tę ifstrukturę w stosie , aby zamiast tego obsługiwać kombinacje z pojedynczą linią:

// All possible combinations of One - Eight.
if (One || TwoOne || ThreeOne || ThreeOneTwo)
    DrawOne;
if (Two || TwoOne || ThreeTwo || ThreeOneTwo)
    DrawTwo;
if (Three || ThreeOne || ThreeTwo || ThreeOneTwo)
    DrawThree;

Stwarza to problem niewiarygodnie długich ocen logicznych, które są mylące w czytaniu i trudne do utrzymania. Po przeredagowaniu tego zacząłem myśleć o alternatywach i pomyślałem o idei switchstruktury z przewrotem między przypadkami.

Muszę użyć gotow tym przypadku, ponieważ C#nie pozwala na awarię. Jednak zapobiega niewiarygodnie długim łańcuchom logicznym, mimo że przeskakuje w switchstrukturze i wciąż powoduje duplikację kodu.

switch (enumVal) {
    case ThreeOneTwo: DrawThree; goto case TwoOne;
    case ThreeTwo: DrawThree; goto case Two;
    case ThreeOne: DrawThree; goto default;
    case TwoOne: DrawTwo; goto default;
    case Two: DrawTwo; break;
    default: DrawOne; break;
}

To wciąż nie jest wystarczająco czyste rozwiązanie, a ze gotosłowem kluczowym wiąże się piętno , którego chciałbym uniknąć. Jestem pewien, że musi być lepszy sposób na oczyszczenie tego.

Moje pytanie

Czy istnieje lepszy sposób obsługi tego konkretnego przypadku bez wpływu na czytelność i łatwość konserwacji?

Uważam, że kod jest trudny do odczytania za pomocą gotoinstrukcji. Poleciłbym ustrukturyzować twoje enuminaczej. Na przykład, jeśli twoje enumpole bitowe reprezentowało jedną z opcji, może wyglądać następująco:

[Flags]
public enum ExampleEnum {
    One = 0b0001,
    Two = 0b0010,
    Three = 0b0100
};

Atrybut Flagi informuje kompilator, że konfigurujesz wartości, które się nie nakładają. Kod wywołujący ten kod może ustawić odpowiedni bit. Następnie możesz zrobić coś takiego, aby wyjaśnić, co się dzieje:

if (myEnum.HasFlag(ExampleEnum.One))
{
    CallOne();
}
if (myEnum.HasFlag(ExampleEnum.Two))
{
    CallTwo();
}
if (myEnum.HasFlag(ExampleEnum.Three))
{
    CallThree();
}

Wymaga to kodu, który konfiguruje się, myEnumaby poprawnie ustawić pola bitowe i oznaczone atrybutem Flagi. Ale możesz to zrobić, zmieniając wartości wyliczeń w przykładzie na:

[Flags]
public enum ExampleEnum {
    One = 0b0001,
    Two = 0b0010,
    Three = 0b0100,
    OneAndTwo = One | Two,
    OneAndThree = One | Three,
    TwoAndThree = Two | Three
};

Kiedy piszesz liczbę w formularzu 0bxxxx, podajesz ją w postaci binarnej. Widać więc, że ustawiamy bit 1, 2 lub 3 (cóż, technicznie 0, 1 lub 2, ale masz pomysł). Możesz także nazwać kombinacje za pomocą bitowego LUB, jeśli kombinacje mogą być często zestawiane razem.

IMO leży u podstaw problemu, ponieważ ten fragment kodu nawet nie powinien istnieć.

Najwyraźniej masz trzy niezależne warunki i trzy niezależne działania, które należy podjąć, jeśli te warunki są prawdziwe. Dlaczego więc to wszystko jest umieszczone w jednym kawałku kodu, który potrzebuje trzech flag boolowskich, aby powiedzieć mu, co ma robić (niezależnie od tego, czy zamieniasz je w wyliczenie), a potem robi jakąś kombinację trzech niezależnych rzeczy? Wydaje się, że zasada jednej odpowiedzialności ma tutaj dzień wolny od pracy.

Umieść wywołania w trzech funkcjach, do których należą (tzn. Gdy odkryjesz potrzebę wykonywania działań) i wyślij kod z tych przykładów do kosza.

Gdyby było dziesięć flag i akcji, a nie trzy, czy rozszerzyłbyś ten rodzaj kodu na 1024 różne kombinacje? Mam nadzieję, że nie! Jeśli 1024 to za dużo, 8 również jest za dużo z tego samego powodu.

Nigdy nie używaj gotos to jedna z koncepcji informatyki „okłamuje dzieci” . To właściwa rada w 99% przypadków, a czasy, kiedy tak nie jest, są tak rzadkie i wyspecjalizowane, że dla wszystkich jest znacznie lepiej, jeśli wyjaśniono to nowym programistom jako „nie używaj ich”.

Kiedy więc powinny być używane? Jest kilka scenariuszy , ale podstawowy wydaje się, że uderzasz: kiedy kodujesz maszynę stanu . Jeśli nie ma lepiej zorganizowanego i ustrukturyzowanego wyrażenia twojego algorytmu niż maszyna stanu, wówczas jego naturalne wyrażenie w kodzie obejmuje nieustrukturyzowane gałęzie i nie można wiele z tym zrobić, co prawdopodobnie nie tworzy struktury sama maszyna stanowa jest bardziej niejasna niż mniejsza.

Autorzy kompilatorów wiedzą o tym, dlatego kod źródłowy większości kompilatorów implementujących parsery LALR ^* zawiera gotos. Jednak bardzo niewiele osób faktycznie koduje własne analizatory leksykalne i analizatory składni.

^{* - IIRC, możliwe jest zaimplementowanie gramatyki LALL w sposób całkowicie rekurencyjny, bez uciekania się do skokowych tabel lub innych nieustrukturyzowanych instrukcji sterujących, więc jeśli jesteś naprawdę przeciwny goto, jest to jedno wyjście.}

Teraz następne pytanie brzmi: „Czy ten przykład jest jednym z tych przypadków?”

Patrząc na to, widzę, że masz trzy różne możliwe kolejne stany, w zależności od przetwarzania bieżącego stanu. Ponieważ jeden z nich („domyślny”) jest po prostu wierszem kodu, technicznie można się go pozbyć, przypinając ten wiersz kodu na końcu stanów, których dotyczy. To doprowadziłoby cię do 2 możliwych następnych stanów.

Jeden z pozostałych („Trzy”) jest rozgałęziony tylko z jednego miejsca, które widzę. Abyś mógł się go pozbyć w ten sam sposób. Otrzymasz kod, który wygląda następująco:

switch (exampleValue) {
    case OneAndTwo: i += 3 break;
    case OneAndThree: i += 4 break;
    case Two: i += 2 break;
    case TwoAndThree: i += 5 break;
    case Three: i += 3 break;
    default: i++ break;
}

Jednak znowu był to podany przez ciebie przykład zabawki. W przypadkach, w których „default” zawiera nietrywialną ilość kodu, „trzy” jest przenoszone z wielu stanów lub (co najważniejsze) dalsze utrzymanie prawdopodobnie doda lub skomplikuje te stany , na pewno byłoby lepiej używając gotos (a być może nawet pozbywając się struktury enum-case, która ukrywa naturę automatu stanu, chyba że istnieje jakiś dobry powód, dla którego musi zostać).

Najlepszą odpowiedzią jest użycie polimorfizmu .

Kolejna odpowiedź, która, według IMO, sprawia, że ​​jeśli wszystko jest bardziej przejrzyste i prawdopodobnie krótsze :

if (One || OneAndTwo || OneAndThree)
  CallOne();
if (Two || OneAndTwo || TwoAndThree)
  CallTwo();
if (Three || OneAndThree || TwoAndThree)
  CallThree();

goto to chyba mój 58 wybór tutaj ...

Dlaczego nie to:

public enum ExampleEnum {
    One = 0, // Why not?
    OneAndTwo,
    OneAndThree,
    Two,
    TwoAndThree,
    Three
}
int[] COUNTS = { 1, 3, 4, 2, 5, 3 }; // Whatever

int ComputeExampleValue(int i, ExampleEnum exampleValue) {
    return i + COUNTS[(int)exampleValue];
}

OK, zgadzam się, to jest hackish (nie jestem deweloperem C # btw, więc przepraszam za kod), ale z punktu widzenia wydajności jest to konieczne? Używanie wyliczeń jako indeksu tablicy jest poprawnym C #.

Jeśli nie możesz lub nie chcesz używać flag, użyj funkcji rekurencyjnej. W trybie 64-bitowym kompilator wygeneruje kod, który jest bardzo podobny do twojej gotoinstrukcji. Po prostu nie musisz sobie z tym poradzić.

int ComputeExampleValue(int i, ExampleEnum exampleValue) {
    switch (exampleValue) {
        case One: return i + 1;
        case OneAndTwo: return ComputeExampleValue(i + 2, ExampleEnum.One);
        case OneAndThree: return ComputeExampleValue(i + 3, ExampleEnum.One);
        case Two: return i + 2;
        case TwoAndThree: return ComputeExampleValue(i + 2, ExampleEnum.Three);
        case Three: return i + 3;
   }
}

Przyjęte rozwiązanie jest w porządku i jest konkretnym rozwiązaniem twojego problemu. Chciałbym jednak postawić na alternatywne, bardziej abstrakcyjne rozwiązanie.

Z mojego doświadczenia wynika, że ​​użycie wyliczeń do zdefiniowania przepływu logiki jest zapachem kodu, ponieważ często jest oznaką złego projektu klasy.

Natrafiłem na prawdziwy przykład tego, co dzieje się w kodzie, nad którym pracowałem w zeszłym roku. Pierwotny programista stworzył jedną klasę, która logowała zarówno import, jak i eksport, i przełączała się między nimi w oparciu o wyliczenie. Teraz kod był podobny i miał trochę zduplikowanego kodu, ale był na tyle inny, że spowodowało to, że kod był znacznie trudniejszy do odczytania i praktycznie niemożliwy do przetestowania. Skończyłem przefakturowanie tego na dwie osobne klasy, co uprościło obie i faktycznie pozwoliło mi wykryć i wyeliminować wiele niezgłoszonych błędów.

Jeszcze raz muszę stwierdzić, że używanie wyliczeń do kontrolowania przepływu logiki jest często problemem projektowym. W ogólnym przypadku Enums powinny być stosowane głównie w celu zapewnienia bezpiecznych dla danego typu, przyjaznych konsumentom wartości, w których możliwe wartości są jasno określone. Są one lepiej wykorzystywane jako właściwość (na przykład jako identyfikator kolumny w tabeli) niż jako mechanizm kontroli logiki.

Rozważmy problem przedstawiony w pytaniu. Naprawdę nie znam tutaj kontekstu ani tego, co reprezentuje ten enum. Czy to losowanie kart? Rysowanie obrazów? Czerpiesz krew? Czy zamówienie jest ważne? Nie wiem też, jak ważna jest wydajność. Jeśli wydajność lub pamięć mają kluczowe znaczenie, to rozwiązanie prawdopodobnie nie będzie tym, czego potrzebujesz.

W każdym razie rozważmy wyliczenie:

// All possible combinations of One - Eight.
public enum ExampleEnum {
    One,
    Two,
    TwoOne,
    Three,
    ThreeOne,
    ThreeTwo,
    ThreeOneTwo
}

Mamy tutaj szereg różnych wartości wyliczeniowych, które reprezentują różne koncepcje biznesowe.

Zamiast tego moglibyśmy zastosować abstrakcje w celu uproszczenia.

Rozważmy następujący interfejs:

public interface IExample
{
  void Draw();
}

Następnie możemy zaimplementować to jako klasę abstrakcyjną:

public abstract class ExampleClassBase : IExample
{
  public abstract void Draw();
  // other common functionality defined here
}

Możemy mieć konkretną klasę reprezentującą rysunek pierwszy, drugi i trzeci (które ze względu na argument mają inną logikę). Mogą potencjalnie korzystać z klasy bazowej zdefiniowanej powyżej, ale zakładam, że koncepcja DrawOne różni się od koncepcji reprezentowanej przez wyliczenie:

public class DrawOne
{
  public void Draw()
  {
    // Drawing logic here
  }
}

public class DrawTwo
{
  public void Draw()
  {
    // Drawing two logic here
  }
}

public class DrawThree
{
  public void Draw()
  {
    // Drawing three logic here
  }
}

A teraz mamy trzy oddzielne klasy, które można skomponować, aby zapewnić logikę dla innych klas.

public class One : ExampleClassBase
{
  private DrawOne drawOne;

  public One(DrawOne drawOne)
  {
    this.drawOne = drawOne;
  }

  public void Draw()
  {
    this.drawOne.Draw();
  }
}

public class TwoOne : ExampleClassBase
{
  private DrawOne drawOne;
  private DrawTwo drawTwo;

  public One(DrawOne drawOne, DrawTwo drawTwo)
  {
    this.drawOne = drawOne;
    this.drawTwo = drawTwo;
  }

  public void Draw()
  {
    this.drawOne.Draw();
    this.drawTwo.Draw();
  }
}

// the other six classes here

To podejście jest bardziej szczegółowe. Ale ma to zalety.

Rozważ następującą klasę, która zawiera błąd:

public class ThreeTwoOne : ExampleClassBase
{
  private DrawOne drawOne;
  private DrawTwo drawTwo;
  private DrawThree drawThree;

  public One(DrawOne drawOne, DrawTwo drawTwo, DrawThree drawThree)
  {
    this.drawOne = drawOne;
    this.drawTwo = drawTwo;
    this.drawThree = drawThree;
  }

  public void Draw()
  {
    this.drawOne.Draw();
    this.drawTwo.Draw();
  }
}

O ile prostsze jest wykrycie brakującego wywołania drawThree.Draw ()? A jeśli kolejność jest ważna, kolejność losowań jest również bardzo łatwa do zobaczenia i do naśladowania.

Wady tego podejścia:

• Każda z ośmiu przedstawionych opcji wymaga osobnej klasy
• To zajmie więcej pamięci
• To sprawi, że Twój kod będzie powierzchownie większy
• Czasami takie podejście nie jest możliwe, chociaż mogą być pewne odmiany

Zalety tego podejścia:

• Każda z tych klas jest całkowicie testowalna; dlatego
• Cyklomatyczna złożoność metod rysowania jest niska (i teoretycznie mogę kpić z klas DrawOne, DrawTwo lub DrawThree, jeśli to konieczne)
• Metody rysowania są zrozumiałe - programista nie musi wiązać mózgu węzłami, pracując nad tym, co robi metoda
• Błędy są łatwe do wykrycia i trudne do napisania
• Klasy składają się na bardziej zaawansowane klasy, co oznacza, że ​​zdefiniowanie klasy ThreeThreeThree jest łatwe

Rozważ to (lub podobne) podejście, ilekroć poczujesz potrzebę zapisania złożonego kodu kontroli logicznej w instrukcjach przypadków. W przyszłości będziesz szczęśliwy, że to zrobiłeś.

Jeśli zamierzasz użyć przełącznika tutaj, Twój kod będzie faktycznie szybszy, jeśli będziesz rozpatrywać każdą sprawę osobno

switch(exampleValue)
{
    case One:
        i++;
        break;
    case Two:
        i += 2;
        break;
    case OneAndTwo:
    case Three:
        i+=3;
        break;
    case OneAndThree:
        i+=4;
        break;
    case TwoAndThree:
        i+=5;
        break;
}

w każdym przypadku wykonywana jest tylko jedna operacja arytmetyczna

także, jak powiedzieli inni, jeśli rozważasz użycie goto, prawdopodobnie powinieneś przemyśleć swój algorytm (chociaż przyznaję, że brak literatury C # może być powodem do użycia goto). Zobacz słynny artykuł Edgara Dijkstry „Idź do oświadczenia uznanego za szkodliwy”

W twoim konkretnym przykładzie, ponieważ wszystko, czego tak naprawdę chciałeś od wyliczenia, to wskaźnik do / do-not dla każdego z kroków, rozwiązanie, które przepisuje twoje trzy ifinstrukcje, jest lepsze niż a switch, i dobrze, że masz zaakceptowaną odpowiedź .

Ale jeśli miałeś bardziej złożoną logikę, która nie zadziałała tak czysto, to nadal uważam, że gotos to switchzdanie jest mylące. Wolałbym zobaczyć coś takiego:

switch (enumVal) {
    case ThreeOneTwo: DrawThree; DrawTwo; DrawOne; break;
    case ThreeTwo:    DrawThree; DrawTwo; break;
    case ThreeOne:    DrawThree; DrawOne; break;
    case TwoOne:      DrawTwo; DrawOne; break;
    case Two:         DrawTwo; break;
    default:          DrawOne; break;
}

To nie jest idealne, ale myślę, że tak jest lepiej niż z gotos. Jeśli sekwencje zdarzeń są tak długie i powielają się tak bardzo, że tak naprawdę nie ma sensu przeliterować pełnej sekwencji dla każdego przypadku, wolę podprogram, niż gotow celu ograniczenia powielania kodu.

Nie jestem pewien, czy ktokolwiek naprawdę ma powód, by nienawidzić słowa kluczowego goto. Jest zdecydowanie archaiczny i nie jest potrzebny w 99% przypadków użycia, ale z jakiegoś powodu jest cechą języka.

Powodem do nienawiści tego gotosłowa kluczowego jest kod

if (someCondition) {
    goto label;
}

string message = "Hello World!";

label:
Console.WriteLine(message);

Ups! To najwyraźniej nie zadziała. messageZmienna nie jest zdefiniowana w tej ścieżce kodu. Więc C # nie przejdzie tego. Ale może to być dorozumiane. Rozważać

object.setMessage("Hello World!");

label:
object.display();

I załóżmy, że displayto zawiera WriteLinestwierdzenie.

Tego rodzaju błąd może być trudny do znalezienia, ponieważ gotozasłania ścieżkę kodu.

To jest uproszczony przykład. Załóżmy, że prawdziwy przykład nie byłby tak oczywisty. Może być pięćdziesiąt linii kodu pomiędzy label:i użyciem message.

Język może pomóc rozwiązać ten problem, ograniczając sposób gotokorzystania z niego, tylko zstępując z bloków. Ale C # gotonie jest tak ograniczony. Może przeskakiwać kod. Ponadto, jeśli zamierzasz ograniczyć goto, równie dobrze jest zmienić nazwę. Inne języki używają breakdo schodzenia z bloków, z liczbą (bloków do wyjścia) lub etykietą (na jednym z bloków).

Pojęcie gototo jest instrukcją języka maszynowego niskiego poziomu. Ale jedynym powodem, dla którego mamy języki wyższego poziomu, jest to, że jesteśmy ograniczeni do abstrakcji wyższego poziomu, np. Zakresu zmiennego.

To powiedziawszy, jeśli użyjesz C # gotowewnątrz switchinstrukcji do przeskakiwania od przypadku do przypadku, jest to w zasadzie nieszkodliwe. Każda sprawa jest już punktem wejścia. Nadal uważam, że nazywanie tego gotojest głupie w tej sytuacji, ponieważ łączy to nieszkodliwe użycie gotoz bardziej niebezpiecznymi formami. Wolałbym, żeby używali continuedo tego czegoś takiego . Ale z jakiegoś powodu zapomnieli mnie zapytać, zanim napisali ten język.

Kiedy masz tak wiele możliwości (a nawet więcej, jak mówisz), to może nie jest to kod, ale dane.

Utwórz słownik odwzorowujący wartości wyliczeniowe na akcje, wyrażone albo jako funkcje, albo jako prostszy typ wyliczenia reprezentujący akcje. Następnie kod można sprowadzić do prostego wyszukiwania w słowniku, a następnie albo wywołując wartość funkcji, albo przełączając uproszczone opcje.

Użyj pętli i różnicy między przerwaniem a kontynuowaniem.

do {
  switch (exampleValue) {
    case OneAndTwo: i += 2; break;
    case OneAndThree: i += 3; break;
    case Two: i += 2; continue;
    case TwoAndThree: i += 2;
    // dropthrough
    case Three: i += 3; continue;
    default: break;
  }
  i++;
} while(0);