„Jak zły” jest niepowiązany kod w bloku try-catch-wreszcie?

Jest to powiązane pytanie: Czy użycie klauzuli w końcu do wykonywania pracy po powrocie jest złe / niebezpieczne?

W przywoływanym Q ostatecznie kod jest związany z zastosowaną strukturą i koniecznością pobierania z wyprzedzeniem. Moje pytanie jest trochę inne i uważam, że jest to istotne dla szerszej publiczności. Moim szczególnym przykładem jest aplikacja C # winform, ale w końcu dotyczyłaby również użycia C ++ / Java.

Zauważam sporo bloków typu try-catch-wreszcie, w których jest dużo kodu niezwiązanego z wyjątkami i obsługą / czyszczeniem wyjątków ukrytych w bloku. Przyznaję, że mam tendencję do posiadania bardzo ścisłych bloków try-catch-wreszcie z kodem ściśle związanym z wyjątkiem i obsługą. Oto kilka przykładów tego, co widzę.

W blokach try będzie wiele wstępnych wywołań i zmiennych, które prowadzą do kodu, który może zostać wygenerowany. Informacje o logowaniu zostaną skonfigurowane i uruchomione również w bloku try.

Na koniec bloki będą miały wywołania formatowania form / module / control (mimo że aplikacja ma się zakończyć, jak wyrażono w bloku catch), a także tworzyć nowe obiekty, takie jak panele.

W przybliżeniu:

    methodName (...)
    {
        próbować
        {
            // Dużo kodu dla metody ...
            // kod, który mógłby wyrzucić ...
            // Dużo więcej kodu dla metody i zwrot ...
        }
        złapać (coś)
        {// obsłuż wyjątek}
        Wreszcie
        {
            // niektóre porządki z powodu wyjątku, zamykanie rzeczy
            // więcej kodu dla rzeczy, które zostały utworzone (ignorując fakt, że mogły zostać zgłoszone wyjątki) ...
            // może stworzyć więcej obiektów
        }
    }

Kod działa, więc ma pewną wartość. Nie jest dobrze zamknięty, a logika jest nieco skomplikowana. Jestem (boleśnie) zaznajomiony z ryzykiem związanym z przesuwaniem kodu i refaktoryzacją, więc moje pytanie sprowadza się do chęci poznania doświadczeń innych osób z podobnie skonstruowanym kodem.

Czy zły styl uzasadnia dokonywanie zmian? Czy ktoś został poważnie poparzony w podobnej sytuacji? Czy chciałbyś podzielić się szczegółami tego złego doświadczenia? Zostaw to, bo jestem przesadnie reagujący i nie jest to taki zły styl? Zyskujesz korzyści związane z utrzymaniem porządku?

Przeżyłem bardzo podobną sytuację, gdy miałem do czynienia ze strasznym, starszym kodem Windows Forms napisanym przez programistów, który wyraźnie nie wiedział, co robią.

Po pierwsze, nie przesadzasz. To jest zły kod. Jak powiedziałeś, blok catch powinien polegać na przerwaniu i przygotowaniu się do zatrzymania. Nie czas na tworzenie obiektów (szczególnie paneli). Nie mogę nawet zacząć wyjaśniać, dlaczego to jest złe.

Biorąc to pod uwagę ...

Moja pierwsza rada: jeśli nie jest zepsuta, nie dotykaj jej!

Jeśli Twoim zadaniem jest utrzymanie kodu, musisz starać się go nie łamać. Wiem, że to bolesne (byłem tam), ale musisz zrobić wszystko, aby nie złamać tego, co już działa.

Moja druga rada brzmi: jeśli musisz dodać więcej funkcji, spróbuj zachować istniejącą strukturę kodu tak bardzo, jak to możliwe, aby nie złamać kodu.

Przykład: jeśli istnieje obrzydliwe zdanie na temat zamiany, które Twoim zdaniem może zostać zastąpione odpowiednim dziedzictwem, musisz być ostrożny i przemyśleć dwa razy, zanim zdecydujesz się przenieść rzeczy.

Na pewno znajdziesz sytuacje, w których refaktoryzacja jest właściwym podejściem, ale uwaga: kod refaktoryzacji jest bardziej podatny na wprowadzanie błędów . Tę decyzję musisz podjąć z perspektywy właścicieli aplikacji, a nie dewelopera. Musisz więc pomyśleć, czy wysiłek (pieniądze) potrzebny do rozwiązania problemu jest wart refaktoryzacji, czy nie. Wielokrotnie widziałem programistę, który spędzał kilka dni naprawiając coś, co tak naprawdę nie jest zepsute tylko dlatego, że uważa, że ​​„kod jest brzydki”.

Moja trzecia rada brzmi: zostaniesz spalony, jeśli złamiesz kod, nie ma znaczenia, czy to twoja wina, czy nie.

Jeśli zostałeś zatrudniony do obsługi technicznej, tak naprawdę nie ma znaczenia, czy aplikacja się rozpada, ponieważ ktoś inny podjął złe decyzje. Z perspektywy użytkownika działało to wcześniej, a teraz go zepsułeś. Ci połamał go!

Joel bardzo dobrze pisze w swoim artykule, wyjaśniając kilka powodów, dla których nie powinieneś przepisywać starszego kodu.

http://www.joelonsoftware.com/articles/fog0000000069.html

Powinieneś czuć się naprawdę źle z tego rodzaju kodem (i nigdy nie powinieneś pisać czegoś takiego), ale utrzymywanie go to zupełnie inny potwór.

O moim doświadczeniu: musiałem utrzymywać kod przez około 1 rok i ostatecznie byłem w stanie przepisać go od zera, ale nie wszystkie naraz.

Stało się tak, że kod był tak zły, że nowych funkcji nie można było wdrożyć. Istniejąca aplikacja miała poważne problemy z wydajnością i użytecznością. W końcu poproszono mnie o zmianę, która zajęłaby mi 3-4 miesiące (głównie dlatego, że praca z tym kodem zajęła mi znacznie więcej czasu niż zwykle). Pomyślałem, że mogę przepisać cały ten utwór (w tym zaimplementować pożądaną nową funkcję) w ciągu około 5-6 miesięcy. Przedstawiłem tę propozycję interesariuszom, którzy zgadzają się ją przepisać (na szczęście dla mnie).

Po przepisaniu tego utworu zrozumieli, że mogę dostarczyć znacznie lepsze rzeczy niż to, co już mieli. Dzięki temu mogłem przepisać całą aplikację.

Moje podejście polegało na przepisaniu go kawałek po kawałku. Najpierw zastąpiłem cały interfejs użytkownika (Windows Forms), następnie zacząłem zastępować warstwę komunikacyjną (wywołania usługi sieci Web), a na koniec zastąpiłem całą implementację serwera (była to gruba aplikacja typu klient / serwer).

Kilka lat później ta aplikacja stała się pięknym kluczowym narzędziem używanym przez całą firmę. Jestem w 100% pewien, że nigdy nie byłoby to możliwe, gdybym nie napisał wszystkiego od nowa.

Mimo że udało mi się to zrobić, ważną częścią jest to, że zainteresowane strony zatwierdziły go i byłem w stanie przekonać ich, że było warte swojej ceny. Tak więc, podczas gdy musisz utrzymywać istniejącą aplikację, staraj się, aby niczego nie zepsuć, a jeśli jesteś w stanie przekonać właścicieli o korzyściach z przepisania, spróbuj zrobić to tak, jak Jack the Ripper: według kawałków.

Jeśli nie ma potrzeby zmiany kodu, pozostaw go bez zmian. Ale jeśli musisz zmienić kod, ponieważ musisz naprawić jakiś błąd lub zmienić funkcjonalność, będziesz musiał go zmienić, jeśli ci się spodoba, czy nie. IMHO jest często bezpieczniejsze i mniej podatne na błędy, jeśli najpierw przekształcisz go na mniejsze, lepiej zrozumiałe części, a następnie dodasz funkcjonalność. Gdy masz pod ręką narzędzia do automatycznego refaktoryzacji, można to zrobić stosunkowo bezpiecznie, z bardzo niewielkim ryzykiem wprowadzenia nowych błędów. I zalecam, abyś otrzymał kopię książki Michaela Feathersa

http://www.amazon.com/Working-Effectively-Legacy-Michael-Feathers/dp/0131177052

który da ci cenne wskazówki, jak uczynić kod bardziej testowalnym, dodaj testy jednostkowe i unikaj łamania kodu podczas dodawania nowych funkcji.

Jeśli zrobisz to dobrze, twoja metoda stanie się na tyle prosta, że ​​try-catch-wreszcie nie będzie już zawierał niepowiązanego kodu w niewłaściwych miejscach.

Załóżmy, że masz sześć metod wywoływania, z których cztery należy wywołać tylko wtedy, gdy pierwsza zakończy się bez błędu. Rozważ dwie alternatywy:

try {
     method1();  // throws
     method2();
     method3();
     method4();
     method5();
 } catch(e) {
     // handle error
 }
 method6();

vs

 try {
      method1();  // throws
 }
 catch(e) {
      // handle error
 }
 if(! error ) {
     method2();
     method3();
     method4();
     method5();
 }
 method6();

W drugim kodzie traktujesz wyjątki jak kody powrotu. Koncepcyjnie jest to identyczne z:

rc = method1();
if( rc != error ) {
     method2();
     method3();
     method4();
     method5();
 }
 method6();

Jeśli zamierzamy zawinąć każdą metodę, która może zgłosić wyjątek w bloku try / catch, to po co w ogóle mieć wyjątki w funkcji języka? Nie ma korzyści i jest więcej pisania.

Powodem, dla którego mamy wyjątki w językach, jest to, że w starych, złych czasach kodów powrotu często mieliśmy sytuacje powyżej, w których mieliśmy wiele metod, które mogły zwracać błędy, a każdy błąd oznaczał całkowite przerwanie wszystkich metod. Na początku mojej kariery, w dawnych czasach C, wszędzie widziałem taki kod:

rc = method1();
if( rc != error ) {
     rc = method2();
     if( rc != error ) {
         rc = method3();
         if( rc != error ) {
             rc = method4();
             if(rc != error ) {
                 method5();
             }
         }
     }
 }
 method6();

Był to niezwykle powszechny wzorzec, który wyjątkowo starannie rozwiązał wyjątki, umożliwiając powrót do pierwszego przykładu powyżej. Reguła stylu, która mówi, że każda metoda ma własny blok wyjątków, całkowicie wyrzuca to przez okno. Dlaczego więc zawracasz sobie głowę?

Zawsze powinieneś starać się, aby blok try zastępował zestaw kodu, który jest koncepcyjnie jednostką. Powinien otaczać kod, dla którego jeśli w jednostce kodowej wystąpił błąd, nie ma sensu uruchamiać żadnego innego kodu w jednostce kodowej.

Wracając do pierwotnego celu wyjątków: pamiętaj, że zostały one utworzone, ponieważ często kod nie jest w stanie łatwo poradzić sobie z błędami w chwili ich wystąpienia. Wyjątkiem jest umożliwienie radzenia sobie z błędami znacznie później w kodzie lub w górę stosu. Ludzie zapominają o tym i w wielu przypadkach łapią je lokalnie, gdy lepiej byłoby po prostu udokumentować, że dana metoda może zgłosić ten wyjątek. Na świecie jest za dużo kodu, który wygląda następująco:

void method0() : throws MyNewException 
{
    try {
        method1();  // throws MyOtherException
    }
    catch(e) {
        if(e == MyOtherException)
            throw MyNewException();
    }
    method2();
}

Tutaj dodajesz tylko warstwy, a warstwy wprowadzają zamieszanie. Po prostu zrób to:

void method0() : throws MyOtherException
{
    method1();
    method2();
}

Poza tym czuję, że jeśli zastosujesz się do tej zasady i znajdziesz więcej niż kilka bloków try / catch w metodzie, powinieneś zapytać, czy sama metoda jest zbyt złożona i czy należy ją podzielić na wiele metod .

Na wynos: blok try powinien zawierać zestaw kodu, który należy przerwać, jeśli w dowolnym miejscu bloku wystąpi błąd. To, czy ten zestaw kodu ma jedną linię, czy tysiąc linii, nie ma znaczenia. (Chociaż jeśli masz tysiąc linii w metodzie, masz inne problemy).