Kilka tygodni temu wziąłem udział w wydarzeniu poświęconym kodzie oprogramowania i jednym z komentarzy było: „Jestem pewien, że wszyscy rozpoznajemy zły kod, gdy go widzimy” i wszyscy kiwnęli mądrze głową bez dalszej dyskusji.
Takie rzeczy zawsze mnie martwią, ponieważ istnieje taki truizm, że wszyscy myślą, że są ponadprzeciętnym kierowcą. Chociaż wydaje mi się, że potrafię rozpoznać zły kod, chciałbym dowiedzieć się więcej o tym, co inni uważają za zapach, ponieważ rzadko jest on szczegółowo omawiany na blogach i tylko w kilku książkach. W szczególności myślę, że byłoby interesujące usłyszeć o wszystkim, co pachnie kodem w jednym języku, ale nie w innym.
Zacznę od łatwego:
Kod w kontroli źródła, który ma duży odsetek skomentowanych kodów - dlaczego on tam jest? czy miał zostać usunięty? czy to w połowie ukończony utwór? może nie powinno to być komentowane i zostało zrobione tylko wtedy, gdy ktoś coś testował? Osobiście uważam, że tego rodzaju rzeczy są naprawdę denerwujące, nawet jeśli jest to tylko dziwna linia tu i tam, ale kiedy widzisz duże bloki przeplatane resztą kodu, jest to całkowicie niedopuszczalne. Zazwyczaj jest to również wskazanie, że reszta kodu może mieć również wątpliwą jakość.
źródło
printf("%c", 7)
zazwyczaj dzwoni mi dzwonek alarmowy. ;)Odpowiedzi:
Zazwyczaj znajduje się w nonsensownym
try..catch
bloku, zwykle przyciąga moją uwagę. Prawie tak dobrze jak/* Not sure what this does, but removing it breaks the build */
.Jeszcze kilka rzeczy:
if
instrukcji złożonychprocess
,data
,change
,rework
,modify
Właśnie znalazłem:
Racja, ponieważ konieczność brutalnej siły połączeń MySQL to właściwy sposób na robienie różnych rzeczy. Okazało się, że w bazie danych wystąpiły problemy z liczbą połączeń, więc przekroczono limit czasu. Zamiast tego debugować, po prostu próbowali raz po raz, aż zadziałało.
źródło
Główną czerwoną flagą dla mnie są zduplikowane bloki kodu, ponieważ pokazuje, że osoba albo nie rozumie podstaw programowania, albo była zbyt wystraszona, aby dokonać odpowiednich zmian w istniejącej bazie kodu.
Zwykłem też liczyć brak komentarzy jako czerwoną flagę, ale ostatnio pracowałem nad bardzo dobrym kodem, bez komentarzy.
źródło
Kod, który próbuje pokazać, jak sprytny jest programista, mimo że nie dodaje żadnej rzeczywistej wartości:
źródło
swap(x, y);
^_^
20 000 funkcji linii (przesada). Każda funkcja, która wymaga więcej niż kilku ekranów, wymaga ponownego faktorowania.
W tym samym wierszu pliki klas, które wydają się trwać wiecznie. Prawdopodobnie istnieje kilka pojęć, które można by podzielić na klasy, które wyjaśniłyby cel i funkcję oryginalnej klasy i prawdopodobnie gdzie jest używana, chyba że wszystkie są metodami wewnętrznymi.
zmienne nieopisowe, nietrywialne lub zbyt wiele trywialnych zmiennych nieopisowych. Sprawiają one, że dedukowanie tego, co się dzieje, jest zagadką.
źródło
Gorzej jest z biblioteki komercyjnej!
źródło
Komentarze, które są tak szczegółowe, że gdyby istniał angielski kompilator, skompilowałby się i działałby idealnie, ale nie opisuje niczego, czego nie koduje.
Ponadto komentarze do kodu, które można było usunąć, gdyby kod był zgodny z kilkoma podstawowymi wytycznymi:
źródło
/
od z*/
, więc cały kod do końca następnego*/
jest ignorowany. Na szczęście podświetlanie składni sprawia, że takie rzeczy są obecnie rzadkością.a
dla user_age? Naprawdę?i = i + 1; //increment i
Kod generujący ostrzeżenia po kompilacji.
źródło
(unsigned int)
niż zaśmiecać moje listy ostrzeżeń / błędów łagodnymi ostrzeżeniami. Nie chciałbym, żeby lista ostrzeżeń stała się martwym punktem. To również znacznie więcej pita wyjaśniający do innych ludzi, dlaczego jesteś ignorując ostrzeżenie, niż jest, aby wyjaśnić, dlaczego robisz obsady naturalnyints
dounsigned ints
.Funkcje z numerami w nazwie zamiast nazw opisowych , takie jak:
Proszę, niech nazwy funkcji coś znaczą! Jeśli doSomething i doSomething2 robią podobne rzeczy, użyj nazw funkcji, które różnicują różnice. Jeśli doSomething2 jest wyłomem w funkcji doSomething, nazwij go swoją funkcjonalnością.
źródło
mshtml
- łamie mi oczy :(Magiczne liczby lub magiczne struny.
źródło
200
drugiej strony ...Może nie najgorszy, ale wyraźnie pokazuje poziom implementatorów:
Jeśli język ma konstrukcję pętli for lub iteratora, wówczas użycie pętli while pokazuje również poziom zrozumienia języka przez implementatora:
Słaba pisownia / gramatyka w dokumentacji / komentarzach zjada prawie tyle samo, co sam kod. Powodem tego jest to, że kod przeznaczony był do czytania przez ludzi i uruchamiania maszyn. Dlatego używamy języków wysokiego poziomu, jeśli twoja dokumentacja jest trudna do przejścia, to zapobiegawczo formuję negatywną opinię o bazie kodu, nie patrząc na nią.
źródło
Ten, który od razu zauważam, to częstotliwość głęboko zagnieżdżonych bloków kodu (if's, while, itp.). Jeśli kod często schodzi głębiej niż na dwa lub trzy poziomy, jest to oznaką problemu projektowego / logicznego. A jeśli dojdzie do głębokości 8 gniazd, niech będzie dobry powód, aby go nie rozpadać.
źródło
return
stwierdzenie, ale gdy powoduje ona ponad 6 poziomów zagnieżdżenia, to myślę, że wyrządza o wiele więcej szkody niż pożytku.Oceniając program ucznia, czasami potrafię powiedzieć w „mrugającym” momencie. Oto natychmiastowe wskazówki:
Rzadko moje pierwsze wrażenia są nieprawidłowe, a te dzwonki ostrzegawcze mają rację przez około 95% czasu . Z jednym wyjątkiem nowy student języka posługiwał się stylem z innego języka programowania. Wkopanie się i ponowne przeczytanie ich stylu w języku innego języka usunęło dla mnie dzwonki alarmowe, a student uzyskał pełne uznanie. Ale takie wyjątki są rzadkie.
Rozważając bardziej zaawansowany kod, oto inne moje ostrzeżenia:
Pod względem stylu generalnie nie lubię widzieć:
Są to tylko wskazówki do złego kodu. Czasami coś, co może się wydawać złym kodem, tak naprawdę nie jest, ponieważ nie znasz intencji programisty. Na przykład może istnieć dobry powód, dla którego coś wydaje się zbyt skomplikowane - może istnieć inna uwaga w grze.
źródło
Osobiste faworyta / peeve: generowane przez IDE nazwiska, które zostają zatwierdzone. Jeśli TextBox1 jest ważną i ważną zmienną w twoim systemie, czeka Cię kolejna rzecz, którą można przejrzeć kod.
źródło
Całkowicie nieużywane zmienne , szczególnie gdy zmienna ma nazwę podobną do używanych zmiennych.
źródło
Wiele osób wspomniało:
Chociaż chciałbym, żeby rzeczy zostały zaimplementowane, przynajmniej zrobili notatkę. To, co uważam za gorsze, to:
Todo są bezwartościowe i mylące, jeśli nigdy nie zawracasz sobie głowy ich usunięciem!
źródło
//TODO
komentarz? Niesamowite!// TODO
, użyj swojego narzędzia do śledzenia błędów, po to jest to!Metoda, która wymaga ode mnie przewinięcia w dół, aby przeczytać wszystko.
źródło
Koniunkcje w nazwach metod:
Wyjaśnienie: Powodem tego jest to, że dzwonki alarmowe wskazują, że metoda prawdopodobnie narusza zasadę pojedynczej odpowiedzialności .
źródło
addEmployee(); updatePayrate();
), to nie sądzę, że to problem.Łączenie oczywiście kodu źródłowego GPL z komercyjnym programem o zamkniętym źródle.
Nie tylko powoduje to bezpośredni problem prawny, ale z mojego doświadczenia wynika, że zwykle oznacza to niedbalstwo lub brak zainteresowania, co znajduje odzwierciedlenie również w innym miejscu kodu.
źródło
Język agnostyczny:
TODO: not implemented
int function(...) { return -1; }
(to samo co „nie zaimplementowano”)0
,-1
lubnull
jako wyjątkowe wartości zwracane.Specyficzne dla języka (C ++):
array new
co najwyraźniej nie jest bezpieczne dla RAII.printf
.Specyficzne dla Microsoft C ++:
Specyficzne dla C ++ / OOP:
źródło
Dziwny styl wcięcia.
Jest kilka bardzo popularnych stylów, a ludzie zabiorą tę debatę do grobu. Ale czasami widzę, że ktoś używa naprawdę rzadkiego, a nawet domowego stylu wcięcia. To znak, że prawdopodobnie nie kodowali z nikim innym niż sobą.
źródło
Używanie dużej liczby bloków tekstowych zamiast wyliczeń lub zmiennych zdefiniowanych globalnie.
Niedobrze:
Lepszy:
Najlepsza:
źródło
Słabo wpisane parametry lub zwracane wartości metod.
źródło
if...else
blok, staje sięif...else if...[...]...else
blokiem$lesseeloginaccountservice
if
stwierdzenia. Przykład z kodu: doif (!($lessee_obj instanceof Lessee && $lessee_obj != NULL))
którego przejechałemif ($lessee_obj == null)
źródło
Zapach kodu: nieprzestrzeganie najlepszych praktyk
Oto krótka wiadomość dla ciebie: 50% światowej populacji jest poniżej średniej inteligencji. Ok, więc niektórzy ludzie mieliby dokładnie przeciętną inteligencję, ale nie bądźmy wybredni. Ponadto jednym z bocznych skutków głupoty jest to, że nie możesz rozpoznać swojej głupoty! Po połączeniu tych oświadczeń rzeczy nie wyglądają tak dobrze.
Wspomniano wiele dobrych rzeczy i ogólnie wydaje się, że nie przestrzega się najlepszych praktyk to zapach kodu.
Najlepsze praktyki zwykle nie są wymyślane losowo i często istnieją z jakiegoś powodu. Wiele razy może to być subiektywne, ale z mojego doświadczenia są w większości uzasadnione. Przestrzeganie najlepszych praktyk nie powinno stanowić problemu, a jeśli zastanawiasz się, dlaczego są takie, jakie są, badaj je, zamiast ignorować i / lub narzekać - może to uzasadnione, a może nie.
Jednym z przykładów najlepszej praktyki może być stosowanie curlies z każdym blokiem if, nawet jeśli zawiera on tylko jedną linię:
Może nie uważasz, że jest to konieczne, ale niedawno przeczytałem , że jest to główne źródło błędów. Zawsze używano nawiasów również omówiono na temat przepełnienia stosu , a sprawdzanie, czy instrukcje if mają nawiasy, jest również regułą w PMD , statycznym analizatorze kodu dla Javy.
Pamiętaj: „Ponieważ to najlepsza praktyka” nigdy nie jest akceptowalną odpowiedzią na pytanie „dlaczego to robisz?” Jeśli nie potrafisz wyrazić, dlaczego coś jest najlepszą praktyką, to nie jest to najlepsza praktyka, to przesąd.
źródło
Komentarze, które mówią: „dzieje się tak, ponieważ konstrukcja podsystemu„ Froz ”jest całkowicie błędna”.
To trwa cały akapit.
Wyjaśniają, że musi nastąpić następujący refaktor.
Ale tego nie zrobiłem.
Być może powiedziano im, że ich szef nie może wówczas zmienić z powodu problemów z czasem lub kompetencjami, ale może dlatego, że ludzie są małostkowi.
Jeśli przełożony uważa, że j. Losowo. programista nie może dokonać refaktoryzacji, wówczas powinien to zrobić przełożony.
W każdym razie tak się dzieje, wiem, że kod został napisany przez podzielony zespół, z możliwą polityką władzy, i nie zmienili projektów podsystemów.
Prawdziwa historia. To mogło zdarzyć się Tobie.
źródło
Czy ktoś może pomyśleć o przykładzie, w którym kod powinien zgodnie z prawem odnosić się do pliku bezwzględną ścieżką?
źródło
/dev/null
i przyjaciele są w porządku. Ale nawet takie rzeczy/bin/bash
są podejrzane - co, jeśli masz jakiś dziwny system, który ma/usr/bin/bash
?/home/tom/dev/randomhacking/thing.wsdl
. To kryminalne, że jest to zachowanie domyślne./dev/null
: Mam nawyk, gdy programuję w systemie Windows, aby utrzymywać aplikacje i bibliotekic:\dev
. Jakoś foldernull
jest zawsze automatycznie tworzony w tym folderze. Przysięgam, nie mam pojęcia, kto to robi. (Jedna z moich ulubionych błędów / funkcji)Łapanie ogólnych wyjątków:
lub
Nadużycie regionu
Zwykle użycie zbyt wielu regionów wskazuje mi, że twoje klasy są za duże. Jest to flaga ostrzegawcza, która sygnalizuje, że powinienem bardziej zbadać ten fragment kodu.
źródło
Konwencje nazewnictwa klas, które pokazują słabe zrozumienie abstrakcji, którą próbują stworzyć. Lub które wcale nie definiują abstrakcji.
Ekstremalny przykład przychodzi mi na myśl w klasie VB, którą kiedyś widziałem, która miała tytuł
Data
i miała ponad 30 000 linii ... w pierwszym pliku. Była to klasa częściowa podzielona na co najmniej pół tuzina innych plików. Większość metod dotyczyła przechowywanych procesów o nazwach podobnych doFindXByYWithZ()
.Nawet z mniej dramatycznymi przykładami, jestem pewien, że wszyscy po prostu „zrzuciliśmy” logikę do źle pomyślanej klasy, nadając jej całkowicie ogólny tytuł, i żałowałem tego później.
źródło
Funkcje, które przywracają podstawową funkcjonalność języka. Na przykład, jeśli kiedykolwiek zobaczysz w JavaScript metodę „getStringLength ()” zamiast wywołania właściwości „.length” ciągu, wiesz, że masz kłopoty.
źródło
Oczywiście bez jakiejkolwiek dokumentacji i okazjonalnych zagnieżdżonego
#define
sźródło