Czy zmienne flagowe są absolutnym złem? [Zamknięte]

47

Czy zmienne flag są złe? Czy poniższe rodzaje zmiennych są głęboko niemoralne i czy niegodziwe jest ich używanie?

„zmienne boolowskie lub całkowite, które przypisujesz wartości w określonych miejscach, a następnie w dół poniżej sprawdzasz, a następnie w innym miejscu, aby coś zrobić, lub nie, na przykład używając newItem = truekilku wierszy poniżej if (newItem ) then


Pamiętam, że wykonałem kilka projektów, w których całkowicie zaniedbałem użycie flag i skończyłem na lepszej architekturze / kodzie; jednak jest to powszechna praktyka w innych projektach, nad którymi pracuję, a gdy kod rośnie i dodaje się flagi, rośnie również kod spaghetti IMHO.

Czy powiedziałbyś, że istnieją przypadki, w których używanie flag jest dobrą praktyką, a nawet konieczne? Czy zgadzasz się, że używanie flag w kodzie to ... czerwone flagi i należy ich unikać / refaktoryzować; ja po prostu radzę sobie z robieniem funkcji / metod, które sprawdzają stany w czasie rzeczywistym.

dukeofgaming
źródło
9
Wygląda na to, że MainMa i ja mamy inną definicję „flagi”. Myślałem o preprocesorze #ifdefs. O który pytałeś?
Karl Bielefeldt,
To jest naprawdę dobre pytanie. Często się nad tym zastanawiałem i rzeczywiście stwierdziłem, że „no cóż, po prostu użyjmy flagi” trochę za dużo.
Paul Richter,
Booleany to flagi. (Tak jak liczby całkowite, takie są ...)
Thomas Eding
7
@KarlBielefeldt Uważam, że OP odnosi się do zmiennych typu boolean lub liczb całkowitych, które przypisujesz wartości w określonych miejscach, a następnie poniżej, a następnie sprawdzasz, czy chcesz coś zrobić, czy nie, na przykład używając newItem = truekilku wierszy poniżejif (newItem ) then
Tulains Córdova
1
Rozważ również wprowadzenie wyjaśnienia refaktoryzacji zmiennych w tym kontekście. Tak długo, jak metoda jest krótka i ma małą liczbę ścieżek, uważam to za prawidłowe zastosowanie.
Daniel B

Odpowiedzi:

41

Problem, który widziałem podczas utrzymywania kodu korzystającego z flag, polega na tym, że liczba stanów rośnie szybko i prawie zawsze występują nieobsługiwane stany. Jeden przykład z własnego doświadczenia: pracowałem nad kodem, który miał te trzy flagi

bool capturing, processing, sending;

Te trzy stworzyły osiem stanów (tak naprawdę były też dwie inne flagi). Kod nie obejmuje wszystkich możliwych kombinacji wartości, a użytkownicy widzieli błędy:

if(capturing && sending){ // we must be processing as well
...
}

Okazało się, że były sytuacje, w których założenie w powyższym stwierdzeniu if było fałszywe.

Flagi z czasem się komplikują i ukrywają rzeczywisty stan klasy. Dlatego należy ich unikać.

Ben
źródło
3
+1, „należy unikać”. Dodałbym coś o „ale flagi są konieczne w niektórych sytuacjach” (niektórzy mogą powiedzieć „konieczne zło”)
Trevor Boyd Smith
2
@TrevorBoydSmith Z mojego doświadczenia wynika, że ​​nie są, potrzebujesz tylko trochę więcej niż przeciętnej mocy mózgu, której
użyłbyś
W twoim przykładzie powinien to być pojedynczy wyliczenie reprezentujące stan, a nie 3 booleany.
user949300
Możesz napotkać podobny problem do tego, z czym się teraz spotykam. Oprócz uwzględnienia wszystkich możliwych stanów, dwie aplikacje mogą mieć tę samą flagę (np. Przesyłanie danych klienta). W takim przypadku tylko jeden program do przesyłania użyje flagi, wyłączy ją i powodzenia w znalezieniu problemu w przyszłości.
Alan
38

Oto przykład, kiedy flagi są przydatne.

Mam kawałek kodu, który generuje hasła (za pomocą kryptograficznie bezpiecznego generatora liczb pseudolosowych). Osoba wywołująca metodę wybiera, czy hasło powinno zawierać wielkie litery, małe litery, cyfry, symbole podstawowe, symbole rozszerzone, symbole greckie, cyrylicy i Unicode.

W przypadku flag wywołanie tej metody jest łatwe:

var password = this.PasswordGenerator.Generate(
    CharacterSet.Digits | CharacterSet.LowercaseLetters | CharacterSet.UppercaseLetters);

i można nawet uprościć:

var password = this.PasswordGenerator.Generate(CharacterSet.LettersAndDigits);

Bez flag, jaki byłby podpis metody?

public byte[] Generate(
    bool uppercaseLetters, bool lowercaseLetters, bool digits, bool basicSymbols,
    bool extendedSymbols, bool greekLetters, bool cyrillicLetters, bool unicode);

o nazwie tak:

// Very readable, isn't it?
// Tell me just by looking at this code what symbols do I want to be included?
var password = this.PasswordGenerator.Generate(
    true, true, true, false, false, false, false, false);

Jak zauważono w komentarzach, innym podejściem byłoby użycie kolekcji:

var password = this.PasswordGenerator.Generate(
    new []
    {
        CharacterSet.Digits,
        CharacterSet.LowercaseLetters,
        CharacterSet.UppercaseLetters,
    });

Jest to o wiele bardziej czytelne w porównaniu do zestawu truei false, ale nadal ma dwie wady:

Główną wadą jest to, że aby umożliwić łączenie wartości, tak CharacterSet.LettersAndDigitsjakbyś pisał coś takiego w Generate()metodzie:

if (set.Contains(CharacterSet.LowercaseLetters) ||
    set.Contains(CharacterSet.Letters) ||
    set.Contains(CharacterSet.LettersAndDigits) ||
    set.Contains(CharacterSet.Default) ||
    set.Contains(CharacterSet.All))
{
    // The password should contain lowercase letters.
}

ewentualnie przepisane w ten sposób:

var lowercaseGroups = new []
{
    CharacterSet.LowercaseLetters,
    CharacterSet.Letters,
    CharacterSet.LettersAndDigits,
    CharacterSet.Default,
    CharacterSet.All,
};

if (lowercaseGroups.Any(s => set.Contains(s)))
{
    // The password should contain lowercase letters.
}

Porównaj to z tym, co masz, używając flag:

if (set & CharacterSet.LowercaseLetters == CharacterSet.LowercaseLetters)
{
    // The password should contain lowercase letters.
}

Druga, bardzo niewielka wada polega na tym, że nie jest jasne, jak zachowałaby się metoda, gdyby została wywołana w następujący sposób:

var password = this.PasswordGenerator.Generate(
    new []
    {
        CharacterSet.Digits,
        CharacterSet.LettersAndDigits, // So digits are requested two times.
    });
Arseni Mourzenko
źródło
10
Wierzę, że OP odnosi się do zmiennych typu boolean lub liczb całkowitych, które przypisujesz wartości w niektórych miejscach, a następnie w dół poniżej sprawdzasz, a następnie w innym celu, aby coś zrobić, lub nie, na przykład używając newItem = truekilku wierszy poniżejif (newItem ) then
Tulains Córdova
1
@MainMa Najwyraźniej jest trzeci: Wersja z 8 logicznymi argumentami to jest to, o czym myślałem, kiedy czytałem „flagi” ...
Izkata
4
Przykro nam, ale IMHO to idealny przypadek dla łączenia łańcuchów metod ( en.wikipedia.org/wiki/Method_chaining ). Dodatkowo możesz użyć tablicy parametrów (musi to być tablica asocjacyjna lub mapa), gdzie dowolny wpis w tej tablicy parametrów pominiesz używa domyślnego zachowania wartości dla tego parametru. Ostatecznie wywołanie za pomocą łączenia łańcuchów metod lub tablic parametrów może być tak zwięzłe i wyraziste jak flagi bitowe, a także, nie każdy język ma operatory bitowe (faktycznie lubię flagi binarne, ale zamiast tego użyłbym metod, które właśnie wspomniałem).
dukeofgaming
3
To nie jest bardzo OOP, prawda? Zrobiłbym interfejs ala: String myNewPassword = makePassword (randomComposeSupplier (new RandomLowerCaseSupplier (), new RandomUpperCaseSupplier (), new RandomNumberSupplier)); z String makePassword (dostawca <Character> charSupplier); oraz Dostawca <Character> randomComposeSupplier (Dostawca <Character> ... dostawcy); Teraz możesz ponownie wykorzystać swoich dostawców do innych zadań, skomponować ich w dowolny sposób i uprościć metodę generatorPassword, aby używała stanu minimalnego.
Dibbeke,
4
@Dibbeke Porozmawiaj o królestwie rzeczowników ...
Phil
15

Ogromnym blokiem funkcyjnym jest zapach, a nie flagi. Jeśli ustawisz flagę na linii 5, sprawdź tylko flagę na linii 354, to źle. Jeśli ustawisz flagę na linii 8 i sprawdzisz flagę na linii 10, nie ma sprawy. Również jedna lub dwie flagi na blok kodu są w porządku, 300 flag w funkcji jest złe.

nbv4
źródło
10

Zazwyczaj flagi można całkowicie zastąpić pewnym smakiem wzorca strategii, z jedną implementacją strategii dla każdej możliwej wartości flagi. To znacznie ułatwia dodawanie nowych zachowań.

W sytuacjach krytycznych pod względem wydajności koszt pośredni może się pojawić i konieczne jest rozłożenie flagi na wyraźne flagi. Biorąc to pod uwagę, mam problem z zapamiętaniem jednego przypadku, w którym musiałem to zrobić.

back2dos
źródło
6

Nie, flagi nie są złe ani zło, które należy za wszelką cenę zrewidować.

Rozważ wywołanie Java Pattern.compile (wyrażenie regularne, flagi int) . Jest to tradycyjna maska ​​bitowa i działa. Spójrz na stałe w java i gdziekolwiek zobaczysz grupę 2 n , wiesz, że są tam flagi.

W idealnym świecie refaktoryzowanym zamiast tego należy użyć EnumSet, w którym stałe są zamiast tego wartościami w wyliczeniu i jak czytamy w dokumentacji:

Wydajność tej klasy w czasie i przestrzeni powinna być wystarczająco dobra, aby umożliwić jej wykorzystanie jako wysokiej jakości, bezpieczną dla użytkownika alternatywę dla tradycyjnych „flag bitowych” opartych na int.

W idealnym świecie to wywołanie Pattern.compile staje się Pattern.compile(String regex, EnumSet<PatternFlagEnum> flags).

Wszystko, co mówiło, nadal jego flagi. O wiele łatwiej jest pracować, Pattern.compile("foo", Pattern.CASE_INSENSTIVE | Pattern.MULTILINE)niż byłoby to mieć, Pattern.compile("foo", new PatternFlags().caseInsenstive().multiline())albo w inny sposób próbować robić to, co flagi naprawdę są i są dobre.

Flagi są często widoczne podczas pracy z rzeczami na poziomie systemu. Kiedy łączysz się z czymś na poziomie systemu operacyjnego, prawdopodobnie gdzieś masz flagę - czy to wartość zwracaną przez proces, czy uprawnienia do pliku, czy flagi do otwierania gniazda. Próba zmiany tych przypadków w polowaniu na czarownice przeciwko wyczuwalnemu zapachowi kodu prawdopodobnie skończy się gorszym kodem, niż gdyby ktoś użył i zaakceptował flagę.

Problem pojawia się, gdy ludzie niewłaściwie używają flag, łącząc je ze sobą i tworząc zestaw frankenflag wszelkiego rodzaju niepowiązanych flag lub próbując użyć ich tam, gdzie wcale nie są flagami.


źródło
5

Zakładam, że mówimy o flagach w podpisach metod.

Używanie pojedynczej flagi jest wystarczająco złe.

Dla twoich kolegów nic to nie znaczy, że pierwszy raz to zobaczą. Będą musieli spojrzeć na kod źródłowy metody, aby ustalić, co ona robi. Prawdopodobnie będziesz w tej samej pozycji kilka miesięcy później, gdy zapomnisz o co chodziło w twojej metodzie.

Przekazywanie flagi do metody zwykle oznacza, że ​​metoda jest odpowiedzialna za wiele rzeczy. Wewnątrz metody prawdopodobnie wykonujesz proste sprawdzenie wierszy:

if (flag)
   DoFlagSet();
else
   DoFlagNotSet();

To słaba separacja problemów i zwykle można to obejść.

Zwykle mam dwie oddzielne metody:

public void DoFlagSet() 
{
}

public void DoFlagNotSet()
{
}

Będzie to bardziej sensowne w przypadku nazw metod, które mają zastosowanie do rozwiązywanego problemu.

Przekazywanie wielu flag jest dwa razy gorsze. Jeśli naprawdę potrzebujesz przekazać wiele flag, rozważ kapsułkowanie ich w klasie. Nawet wtedy nadal będziesz mieć do czynienia z tym samym problemem, ponieważ Twoja metoda prawdopodobnie robi wiele rzeczy.

CodeART
źródło
3

Flagi i większość zmiennych temp ma silny zapach. Najprawdopodobniej można je refaktoryzować i zastąpić metodami zapytań.

Ulepszony:

Flagi i zmienne tymczasowe podczas wyrażania stanu, powinny zostać ponownie przekształcone w metody zapytań. Wartości stanu (booleany, inty i inne prymitywy) powinny prawie zawsze być ukryte jako część szczegółów implementacji.

Flagi używane do sterowania, routingu i ogólnego przebiegu programu mogą również wskazywać na możliwość przefakturowania sekcji struktur kontrolnych na osobne strategie lub fabryki lub cokolwiek, co może być odpowiednie w danej sytuacji, które nadal korzystają z metod zapytań.

JustinC
źródło
2

Kiedy mówimy o flagach, powinniśmy wiedzieć, że zostaną zmodyfikowane w czasie wykonywania programu i że wpłyną na zachowanie programu na podstawie ich stanów. Dopóki będziemy mieć czystą kontrolę nad tymi dwiema rzeczami, będą działać świetnie.

Flagi mogą działać świetnie, jeśli

  • Zdefiniowałeś je w odpowiednim zakresie. Przez odpowiedni rozumiem, zakres nie powinien zawierać żadnego kodu, który nie musi / nie powinien ich modyfikować. Lub przynajmniej kod jest bezpieczny (na przykład nie można go wywoływać bezpośrednio z zewnątrz)
  • Jeśli zachodzi potrzeba obsługi flag z zewnątrz i jeśli jest wiele flag, możemy zakodować moduł obsługi flag jako jedyny sposób bezpiecznego modyfikowania flag. Ten moduł obsługi flag może sam kapsułkować flagi i metody ich modyfikowania. Można go następnie utworzyć jako singleton, a następnie podzielić na klasy, które potrzebują dostępu do flag.
  • I wreszcie dla utrzymania, jeśli jest zbyt wiele flag:
    • Nie trzeba mówić, że powinni stosować rozsądne nazewnictwo
    • Powinny być udokumentowane prawidłowymi wartościami (mogą być z wyliczeniami)
    • Powinny być udokumentowane za pomocą KODU, KTÓRY ZMODYFIKUJE każdy z nich, a także za pomocą którego WARUNKU spowoduje przypisanie określonej wartości do flagi.
    • KTÓRY KOD JEST ZUŻYCIE, a JAKIE ZACHOWANIA przyniosą określoną wartość

Jeśli jest dużo piekła flag, należy poprzedzić dobrą pracę projektową, ponieważ flagi zaczynają odgrywać kluczową rolę w zachowaniu programu. Możesz przejść do diagramów stanu do modelowania. Takie diagramy działają również jako dokumentacja i wskazówki wizualne podczas ich obsługi.

Dopóki te rzeczy są na miejscu, myślę, że nie doprowadzi to do bałaganu.

Mahesha999
źródło
1

Z pytania wynikało, że QA oznacza zmienne flagowe (globalne), a nie bity parametru funkcji.

Są sytuacje, w których nie masz wielu innych możliwości. Na przykład bez systemu operacyjnego musisz ocenić przerwania. Jeśli przerwanie przychodzi bardzo często i nie masz czasu na przeprowadzenie długiej oceny w ISR, nie tylko jest dozwolone, ale czasem nawet najlepsze praktyki, aby ustawić tylko niektóre flagi globalne w ISR (powinieneś spędzić jak najmniej czasu) w ISR) i do oceny tych flag w głównej pętli.

vsz
źródło
0

Nigdy nie sądzę, żeby coś było absolutnym złem w programowaniu.

Jest jeszcze jedna sytuacja, w której flagi mogą być w porządku, o których tu jeszcze nie wspomniano ...

Rozważ użycie zamknięć w tym fragmencie kodu JavaScript:

exports.isPostDraft = function ( post, draftTag ) {
  var isDraft = false;
  if (post.tags)
    post.tags.forEach(function(tag){ 
      if (tag === draftTag) isDraft = true;
    });
  return isDraft;
}

Funkcja wewnętrzna, przekazana do „Array.forEach”, nie może po prostu „zwrócić true”.

Dlatego musisz utrzymywać stan na zewnątrz za pomocą flagi.

firstdoit
źródło