Zapach kodu jest objawem wskazującym na problem w projekcie, który potencjalnie może zwiększyć liczbę błędów: nie dotyczy to regionów, ale regiony mogą przyczyniać się do tworzenia zapachów kodu, takich jak długie metody.
Od:
Anty-wzorzec (lub anty-wzór) to wzorzec stosowany w operacjach społecznych lub biznesowych lub inżynierii oprogramowania, który może być powszechnie stosowany, ale w praktyce jest nieskuteczny i / lub przynosi efekt przeciwny do zamierzonego
regiony są anty-wzorami. Wymagają więcej pracy, która nie poprawia jakości ani czytelności kodu, co nie zmniejsza liczby błędów, a może tylko komplikować kod do refaktoryzacji.
Nie używaj regionów wewnątrz metod; zamiast tego refaktoryzuj
Metody muszą być krótkie . Jeśli w metodzie jest tylko dziesięć linii, prawdopodobnie nie użyłbyś regionów do ukrycia pięciu z nich podczas pracy z innymi pięcioma.
Ponadto każda metoda musi robić jedną i jedyną rzecz . Z drugiej strony regiony mają na celu oddzielenie różnych rzeczy . Jeśli twoja metoda wykonuje A, to B, logiczne jest utworzenie dwóch regionów, ale jest to złe podejście; zamiast tego powinieneś przeredagować metodę na dwie osobne metody.
Zastosowanie regionów w tym przypadku może również utrudnić refaktoryzację. Wyobraź sobie, że masz:
private void DoSomething()
{
var data = LoadData();
#region Work with database
var verification = VerifySomething();
if (!verification)
{
throw new DataCorruptedException();
}
Do(data);
DoSomethingElse(data);
#endregion
#region Audit
var auditEngine = InitializeAuditEngine();
auditEngine.Submit(data);
#endregion
}
Zwinięcie pierwszego regionu w celu skoncentrowania się na drugim jest nie tylko ryzykowne: możemy łatwo zapomnieć o wyjątku zatrzymującym przepływ ( returnzamiast tego mogłaby istnieć klauzula ochronna , co jest jeszcze trudniejsze do zauważenia), ale również miałoby problem jeśli kod powinien być refaktoryzowany w ten sposób:
private void DoSomething()
{
var data = LoadData();
#region Work with database
var verification = VerifySomething();
var info = DoSomethingElse(data);
if (verification)
{
Do(data);
}
#endregion
#region Audit
var auditEngine = InitializeAuditEngine(info);
auditEngine.Submit(
verification ? new AcceptedDataAudit(data) : new CorruptedDataAudit(data));
#endregion
}
Teraz regiony nie mają sensu i nie można odczytać i zrozumieć kodu w drugim regionie bez spojrzenia na kod w pierwszym.
Innym przypadkiem, który czasem widzę, jest ten:
public void DoSomething(string a, int b)
{
#region Validation of arguments
if (a == null)
{
throw new ArgumentNullException("a");
}
if (b <= 0)
{
throw new ArgumentOutOfScopeException("b", ...);
}
#endregion
#region Do real work
...
#endregion
}
Kuszące jest używanie regionów, gdy sprawdzanie poprawności argumentów zaczyna się od dziesiątek LOC, ale istnieje lepszy sposób na rozwiązanie tego problemu: ten używany przez kod źródłowy .NET Framework:
public void DoSomething(string a, int b)
{
if (a == null)
{
throw new ArgumentNullException("a");
}
if (b <= 0)
{
throw new ArgumentOutOfScopeException("b", ...);
}
InternalDoSomething(a, b);
}
private void InternalDoSomething(string a, int b)
{
...
}
Nie używaj regionów poza metodami do grupowania
Niektóre osoby używają ich do grupowania pól, właściwości itp. Takie podejście jest błędne: jeśli kod jest zgodny z StyleCop, to pola, właściwości, metody prywatne, konstruktory itp. Są już zgrupowane i łatwe do znalezienia. Jeśli nie, nadszedł czas, aby zacząć myśleć o stosowaniu reguł, które zapewnią jednolitość w całej bazie kodu.
Inne osoby używają regionów, aby ukryć wiele podobnych podmiotów . Na przykład, jeśli masz klasę zawierającą sto pól (co daje co najmniej 500 linii kodu, jeśli policzysz komentarze i białe znaki), możesz ulec pokusie, aby umieścić te pola w regionie, zwinąć je i zapomnieć o nich. Ponownie robisz to źle: mając tak wiele pól w klasie, powinieneś lepiej pomyśleć o zastosowaniu dziedziczenia lub pokroić obiekt na kilka obiektów.
Wreszcie, niektórzy ludzie mają pokusę, aby użyć regionów do grupowania powiązanych ze sobą rzeczy : zdarzenia z jego delegatem lub metody związanej z IO z innymi metodami związanymi z IO itp. W pierwszym przypadku staje się bałaganem, który jest trudny do utrzymania , Przeczytaj i zrozum. W drugim przypadku lepszym rozwiązaniem byłoby prawdopodobnie utworzenie kilku klas.
Czy regiony mają dobre zastosowanie?
Nie. Nie było użycie starszego typu: kod wygenerowany. Mimo to narzędzia do generowania kodu muszą po prostu używać klas częściowych. Jeśli C # ma obsługę regionów, to głównie dlatego, że używa tego starszego typu, a ponieważ zbyt wiele osób używa regionów w swoim kodzie, nie będzie możliwe ich usunięcie bez zerwania istniejących baz kodów.
Pomyśl o tym jak o goto. Fakt, że język lub IDE obsługuje funkcję, nie oznacza, że należy jej używać codziennie. Reguła StyleCop SA1124 jest jasna: nie należy używać regionów. Nigdy.
Przykłady
Obecnie dokonuję przeglądu kodu mojego współpracownika. Baza kodów zawiera wiele regionów i jest właściwie doskonałym przykładem zarówno tego, jak nie używać regionów, jak i dlaczego regiony prowadzą do złego kodu. Oto kilka przykładów:
4000 potworów LOC:
Niedawno przeczytałem gdzieś na Programmers.SE, że gdy plik zawiera zbyt wiele usings (po wykonaniu polecenia „Usuń nieużywane zastosowania”), to dobry znak, że klasa wewnątrz tego pliku robi zbyt wiele. To samo dotyczy rozmiaru samego pliku.
Podczas przeglądania kodu natrafiłem na plik LOC o wielkości 4000. Okazało się, że autor tego kodu po prostu skopiował i wkleił tę samą 15-liniową metodę setki razy, nieznacznie zmieniając nazwy zmiennych i wywoływaną metodę. Prosty regex pozwolił przyciąć plik z 4 000 LOC do 500 LOC, po prostu dodając kilka ogólnych; Jestem całkiem pewien, że dzięki bardziej sprytnemu refaktoryzacji ta klasa może zostać zredukowana do kilkudziesięciu wierszy.
Korzystając z regionów, autor zachęcił się do zignorowania faktu, że kod jest niemożliwy do utrzymania i źle napisany, a także do znacznego skopiowania kodu zamiast go przefiltrować.
Region „Do A”, Region „Do B”:
Innym doskonałym przykładem była metoda inicjalizacji potwora, która po prostu wykonała zadanie 1, następnie zadanie 2, następnie zadanie 3 itd. Było pięć lub sześć zadań, które były całkowicie niezależne, każde inicjując coś w klasie kontenera. Wszystkie te zadania zostały zgrupowane w jedną metodę i zgrupowane w regiony.
Miało to jedną zaletę:
- Metoda była dość jasna do zrozumienia, patrząc na nazwy regionów. To powiedziawszy, ta sama metoda raz zrefaktoryzowana byłaby tak samo jasna jak oryginał.
Z drugiej strony problemy były liczne:
Nie było oczywiste, czy istnieją zależności między regionami. Mamy nadzieję, że nie było ponownego użycia zmiennych; w przeciwnym razie utrzymanie może być koszmarem jeszcze bardziej.
Metoda była prawie niemożliwa do przetestowania. Skąd miałbyś łatwo wiedzieć, czy metoda, która wykonuje dwadzieścia rzeczy na raz, robi to poprawnie?
Region pól, region właściwości, region konstruktora:
Sprawdzony kod zawierał także wiele regionów grupujących wszystkie pola razem, wszystkie właściwości razem itp. To miał oczywisty problem: wzrost kodu źródłowego.
Kiedy otworzysz plik i zobaczysz ogromną listę pól, będziesz bardziej skłonny do refaktoryzacji klasy, a następnie pracy z kodem. W regionach masz nawyk załamywania się i zapominania o tym.
Innym problemem jest to, że jeśli robisz to wszędzie, tworzysz regiony jednobryłowe, co nie ma żadnego sensu. Tak było w przypadku kodu, który sprawdziłem, gdzie było wiele #region Constructorzawierających jednego konstruktora.
Wreszcie pola, właściwości, konstruktory itp. Powinny już być w porządku . Jeśli są i są zgodne z konwencjami (stałe zaczynające się od dużej litery itp.), Jest już jasne, gdzie na typie elementów zatrzymuje się, a inne zaczynają, więc nie trzeba jawnie tworzyć dla tego regionów.