Eliminowanie magicznych liczb: kiedy należy powiedzieć „nie”?

36

Wszyscy wiemy, że magiczne liczby (wartości zakodowane na stałe) mogą siać spustoszenie w twoim programie, szczególnie gdy nadszedł czas na modyfikację części kodu, która nie ma komentarzy, ale gdzie wytyczasz linię?

Na przykład, jeśli masz funkcję, która oblicza liczbę sekund między dwoma dniami, czy zastępujesz

seconds = num_days * 24 * 60 * 60

z

seconds = num_days * HOURS_PER_DAY * MINUTES_PER_HOUR * SECONDS_PER_MINUTE

W którym momencie zdecydujesz, że jest całkowicie oczywiste, co oznacza wartość zakodowana na stałe, i zostaw to w spokoju?

refactoring coding-standards maintainability

— oosterwal
źródło

2

Zastąp to obliczenie funkcją lub makrem, aby Twój kod wyglądał jakseconds = CALC_SECONDS(num_days);

— FrustratedWithFormsDesigner

15

TimeSpan.FromDays(numDays).Seconds;

— Nikt

18

@oosterwal: Dzięki tej postawie ( HOURS_PER_DAY will never need to be altered) nigdy nie będziesz kodować oprogramowania wdrażanego na Marsie. : P

— FrustratedWithFormsDesigner

23

Zmniejszyłbym liczbę stałych do zaledwie SECONDS_PER_DAY = 86400. Po co obliczać coś, co się nie zmieni?

— JohnFx,

17

Co z sekundami przestępnymi?

— John

40

Istnieją dwa powody, aby używać stałych symbolicznych zamiast literałów numerycznych:

Aby uprościć konserwację, jeśli zmienią się magiczne liczby. Nie dotyczy to twojego przykładu. Jest bardzo mało prawdopodobne, aby zmieniła się liczba sekund w ciągu godziny lub liczba godzin w ciągu dnia.
Aby poprawić czytelność. Wyrażenie „24 * 60 * 60” jest dość oczywiste dla prawie wszystkich. „SECONDS_PER_DAY” też, ale jeśli polujesz na błąd, być może będziesz musiał sprawdzić, czy SECONDS_PER_DAY został poprawnie zdefiniowany. W skrócie jest wartość.

W przypadku liczb magicznych, które pojawiają się dokładnie raz i są niezależne od reszty programu, decyzja o utworzeniu symbolu dla tej liczby jest kwestią gustu. W razie wątpliwości idź dalej i stwórz symbol.

Nie rób tego:

public static final int THREE = 3;

— Kevin Cline
źródło

3

+1 @kevin cline: Zgadzam się z twoją opinią na temat zwięzłości w poszukiwaniu błędów. Dodatkową korzyścią, jaką widzę przy użyciu nazwanej stałej, szczególnie podczas debugowania, jest to, że jeśli okaże się, że stała została niepoprawnie zdefiniowana, wystarczy zmienić tylko jeden fragment kodu zamiast przeszukiwać cały projekt pod kątem wszystkich wystąpień nieprawidłowo zaimplementowanej wartość.

— oosterwal

40

Lub jeszcze gorzej:publid final int FOUR = 3;

— gablin

3

Och, kochanie, musiałeś pracować z tym samym facetem, z którym kiedyś pracowałem.

— Szybko_ teraz

2

@gablin: Aby być uczciwym, puby mają dość pokrywek.

— Alan Pearce

10

Widziałem to: public static int THREE = 3;... uwaga - nie final!

— Stephen C,

29

Przestrzegałbym zasady, by nigdy nie mieć magicznych liczb.

Podczas

seconds = num_days * 24 * 60 * 60

Jest doskonale czytelny przez większość czasu, po kodowaniu przez 10 godzin dziennie przez trzy lub cztery tygodnie w trybie crunch

seconds = num_days * HOURS_PER_DAY * MINUTES_PER_HOUR * SECONDS_PER_MINUTE

jest znacznie łatwiejszy do odczytania.

Sugestia FrustratedWithFormsDesigner jest lepsza:

seconds = num_days * DAYS_TO_SECOND_FACTOR

a nawet lepiej

seconds = CONVERT_DAYS_TO_SECONDS(num_days)

Rzeczy przestają być oczywiste, kiedy jesteś bardzo zmęczony. Kod defensywnie .

— Vitor Py
źródło

13

Wejście w tryb chrupania, jak to opisujesz, przynosi efekt przeciwny do zamierzonego, którego należy unikać. Programiści osiągają szczytową wydajność przy około 35-40 godzinach tygodniowo.

— btilly,

4

@btilly Z całego serca się z tobą zgadzam. Ale zdarza się, często z powodu czynników zewnętrznych.

— Vitor Py

3

Ogólnie definiuję stałe dla sekundy, minuty, dnia i godziny. Jeśli nic więcej „30 * MINUTA” jest naprawdę łatwe do odczytania i wiem, że nadszedł czas, nie muszę o tym myśleć.

— Zachary K

9

@btilly: Szczyt wynosi 35-40 godzin lub poziom alkoholu we krwi od 0,129% do 0,138%. Przeczytałem to na XKCD , więc to musi być prawda!

— oosterwal

1

Gdybym zobaczył stałą, taką jak HOURS_PER_DAY, usunęłbym ją, a następnie publicznie upokorzyłem przed innymi. Ok, może zrezygnowałbym z publicznego upokorzenia, ale prawdopodobnie usunę go.

— Ed S.

8

Czas powiedzieć „nie” prawie zawsze. Czasy, w których uważam, że łatwiej jest używać liczb zakodowanych na stałe, są w miejscach takich jak układ interfejsu użytkownika - tworzenie stałej dla pozycjonowania każdej kontrolki w formularzu staje się bardzo męczące i męczące, a jeśli ten kod jest zwykle obsługiwany przez projektanta interfejsu użytkownika, nie ma większego znaczenia. ... chyba że interfejs użytkownika jest ułożony dynamicznie lub nie używa względnych pozycji do jakiejś kotwicy lub jest pisany ręcznie. W takim przypadku powiedziałbym, że lepiej jest zdefiniować pewne znaczące stałe dla układu. A jeśli potrzebujesz współczynnika kruszenia tu lub tam, aby wyrównać / ustawić coś „w sam raz”, to również należy zdefiniować.

Ale w twoim przykładzie uważam, że zastąpienie 24 * 60 * 60przez DAYS_TO_SECONDS_FACTORjest lepsze.

Przyznaję, że wartości zakodowane są również OK, gdy kontekst i użycie są całkowicie jasne. Jest to jednak wezwanie do osądu ...

Przykład:

Jak wskazał @rmx, użycie 0 lub 1 w celu sprawdzenia, czy lista jest pusta, czy może w granicach pętli jest przykładem przypadku, w którym cel stałej jest bardzo jasny.

— FrustratedWithFormsDesigner
źródło

2

Zwykle można używać 0lub, jak 1sądzę, używać . if(someList.Count != 0) ...jest lepszy niż if(someList.Count != MinListCount) .... Nie zawsze, ale ogólnie.

— Nikt

2

@Dima: VS projektant formularzy obsługuje to wszystko. Jeśli chce tworzyć stałe, nie mam nic przeciwko. Ale ja nie wchodząc wygenerowanego kodu i zastąpienie wszystkie stałe wartości ze stałych.

— FrustratedWithFormsDesigner

4

Nie mylmy jednak kodu przeznaczonego do generowania i obsługiwania przez narzędzie z kodem napisanym do spożycia przez ludzi.

— biziclop,

1

@FrustratedWithFormsDesigner, jak zauważył @biziclop, generowany kod jest zupełnie innym zwierzęciem. Stałe nazwane bezwzględnie muszą być używane w kodzie odczytywanym i modyfikowanym przez ludzi. Wygenerowany kod, przynajmniej w idealnym przypadku, nie powinien być w ogóle modyfikowany.

— Dima,

2

@FrustratedWithFormsDesigner: Co się dzieje, gdy masz dobrze znaną wartość zakodowaną w dziesiątkach plików w swoim programie, która nagle musi zostać zmieniona? Na przykład, kodujesz na stałe wartość reprezentującą liczbę tyknięć zegara na mikrosekundę dla wbudowanego procesora, a następnie mówi się o przeniesieniu oprogramowania do projektu, w którym istnieje inna liczba tyknięć zegara na mikrosekundę. Jeśli twoja wartość była czymś powszechnym, np. 8, wykonanie wyszukiwania / zamiany na dziesiątkach plików może spowodować więcej problemów.

— oosterwal

8

Przestań, gdy nie możesz przypisać znaczenia ani celu numerowi.

seconds = num_days * HOURS_PER_DAY * MINUTES_PER_HOUR * SECONDS_PER_MINUTE

jest znacznie łatwiejszy do odczytania niż zwykłe używanie liczb. (Chociaż można by to uczynić bardziej czytelnym dzięki pojedynczej SECONDS_PER_DAYstałej, ale jest to zupełnie osobny problem).

Załóżmy, że programista patrząc na kod może zobaczyć, co robi. Ale nie zakładaj, że oni również wiedzą dlaczego. Jeśli twoja stała pomaga zrozumieć, dlaczego? Jeśli nie, nie rób tego.

Jeśli skończy się zbyt wiele stałych, jak sugerowała jedna odpowiedź, rozważ użycie zewnętrznego pliku konfiguracyjnego, ponieważ posiadanie dziesiątek stałych w pliku nie poprawia dokładnie czytelności.

— biziclop
źródło

7

Prawdopodobnie powiedziałbym „nie” takim rzeczom jak:

#define HTML_END_TAG "</html>"

I zdecydowanie powiedziałbym „nie”:

#define QUADRATIC_DISCRIMINANT_COEF 4
#define QUADRATIC_DENOMINATOR_COEF  2

— dan04
źródło

7

Jednym z najlepszych przykładów, jakie znalazłem w celu promowania używania stałych do oczywistych rzeczy, takich jak HOURS_PER_DAY:

Obliczaliśmy, jak długo rzeczy leżą w kolejce do pracy. Wymagania zostały luźno zdefiniowane, a programator zakodowany 24na stałe w wielu miejscach. W końcu zdaliśmy sobie sprawę, że niesprawiedliwe jest karanie użytkowników za siedzenie na problemie przez 24 godziny, podczas gdy naprawdę działa tylko przez 8 godzin dziennie. Gdy przyszło zadanie, aby to naprawić ORAZ zobaczyć, jakie inne raporty mogą mieć ten sam problem, bardzo trudno było grep / przeszukać kod 24, byłoby znacznie łatwiej grep / przeszukaćHOURS_PER_DAY

— bkulyk
źródło

och nie, więc godziny dziennie różnią się w zależności od tego, czy odnoszą się do godzin pracy dziennie (pracuję 7,5 BTW) czy godzin dziennie. Aby zmienić znaczenie stałej w ten sposób, należy zastąpić jej nazwę inną nazwą. Twój punkt na temat łatwego wyszukiwania jest jednak ważny.

— gbjbaanb

2

Wygląda na to, że w tym przypadku HOURS_PER_DAY nie był tak naprawdę pożądaną stałą. Ale możliwość wyszukiwania według nazwy jest ogromną korzyścią, nawet jeśli (lub zwłaszcza jeśli) musisz zmienić ją na coś innego w wielu miejscach.

— David K

4

Myślę, że dopóki liczba jest całkowicie stała i nie ma możliwości zmiany, jest całkowicie akceptowalna. Więc w twoim przypadku seconds = num_days * 24 * 60 * 60jest w porządku (zakładając oczywiście, że nie robisz czegoś głupiego jak wykonywanie tego rodzaju obliczeń w pętli) i jest prawdopodobnie lepsza dla czytelności niż seconds = num_days * HOURS_PER_DAY * MINUTES_PER_HOUR * SECONDS_PER_MINUTE.

Kiedy robisz takie rzeczy, to źle:

lineOffset += 24; // 24 lines to a page

Nawet jeśli nie możesz już zmieścić wierszy na stronie lub nawet jeśli nie masz zamiaru go zmieniać, użyj zamiast tego stałej zmiennej, ponieważ pewnego dnia wróci cię prześladować. Ostatecznie chodzi o czytelność, a nie oszczędzenie 2 cykli obliczeń na CPU. To nie jest już rok 1978, kiedy cenne bajty zostały wyciśnięte z całej ich wartości.

— Neil
źródło

2

Czy uważasz, że dopuszczalne jest zakodowanie niezmiennej wartości 86400 zamiast używania stałej o nazwie SECONDS_PER_DAY? W jaki sposób sprawdziłbyś, czy wszystkie wystąpienia wartości były poprawne i na przykład nie brakowało 0, czy nie zamieniałem 6 reklam 4?

— oosterwal

Dlaczego więc nie: sekundy = liczba_dni * 86400? To też się nie zmieni.

— JeffO,

2

Zrobiłem SECONDS_PER_DAY na dwa sposoby - używając nazwy i numeru. Gdy wrócisz do kodu 2 lata później, podany numer ZAWSZE ma sens.

— Szybko_ teraz

2

sekundy = liczba_dni * 86400 nie jest dla mnie jasne. To się ostatecznie liczy. Jeśli widzę „sekundy = liczba_dni * 24 * 60 * 60”, poza tym, że nazwy zmiennych nadają sens dość dobrze w tym przypadku, natychmiast zadałbym sobie pytanie, dlaczego je rozdzieliłem, a znaczenie staje się oczywiste, ponieważ odszedłem je jako liczby (stąd są stałe), a nie zmienne, do których dalsze badanie wymagałoby ode mnie zrozumienia ich wartości i jeśli są stałe.

— Neil

1

Co ludzie często nie zdają sobie sprawy: jeśli zmienisz tę wartość parametru Offset z 24 na 25, będziesz musiał przejść przez cały kod, aby zobaczyć, gdzie 24 zostało użyte i czy wymaga zmiany, a następnie obliczenia wszystkich dni do godzin pomnożenie przez 24 naprawdę przeszkadza.

— gnasher729

3

seconds = num_days * 24 * 60 * 60

Jest całkowicie w porządku. To nie są tak naprawdę magiczne liczby, ponieważ nigdy się nie zmienią.

Wszelkie liczby, które mogą się w uzasadniony sposób zmienić lub nie mają oczywistego znaczenia, powinny być umieszczone w zmiennych Co oznacza prawie wszystkie z nich.

— Carra
źródło

2

Czy seconds = num_days * 86400nadal byłby do przyjęcia? Gdyby taka wartość była używana wielokrotnie w wielu różnych plikach, w jaki sposób sprawdziłbyś, czy ktoś nie przypadkowo wpisał seconds = num_days * 84600w jednym lub dwóch miejscach?

— oosterwal

1

Pisanie 86400 różni się bardzo od pisania 24 * 60 * 60.

— Carra

4

Oczywiście to się zmieni. Nie każdego dnia ma 86 400 sekund. Weźmy na przykład czas letni. Raz w roku niektóre miejsca mają tylko 23 godziny dziennie, a innego dnia będą miały 25. kupę twoich numerów są zepsute.

— Dave DeLong

1

@Dave, doskonały punkt. Istnieją sekundy przestępne - en.wikipedia.org/wiki/Leap_second

Uczciwy punkt. Dodanie funkcji byłoby zabezpieczeniem, jeśli kiedykolwiek będziesz musiał złapać te wyjątki.

— Carra,

3

Unikałbym tworzenia stałych (wartości magicznych) w celu konwersji wartości z jednej jednostki na drugą. W przypadku konwersji wolę nazwę metody mówionej. W tym przykładzie byłoby to np. DayToSeconds(num_days)Wewnętrzne, że metoda nie potrzebuje magicznych wartości, ponieważ znaczenie „24” i „60” jest jasne.

W takim przypadku nigdy nie użyłbym sekund / minut / godzin. Używałbym tylko TimeSpan / DateTime.

— KayS
źródło

1

Użyj kontekstu jako parametru do podjęcia decyzji

Na przykład masz funkcję o nazwie „replaceSecondsBetween: aDay and: anotherDay”, nie będziesz musiał wiele wyjaśniać, co robią te liczby, ponieważ nazwa funkcji jest dość reprezentatywna.

Kolejne pytanie brzmi: jakie są możliwości obliczenia go w inny sposób? Czasami istnieje wiele sposobów na zrobienie tego samego, więc aby pomóc przyszłym programistom i pokazać im, jakiej metody użyłeś, zdefiniowanie stałych może pomóc to rozgryźć.

— guiman
źródło