Czy powinienem refaktoryzować duże funkcje, które w większości składają się z jednego wyrażenia regularnego? [Zamknięte]

Właśnie napisałem funkcję, która obejmuje około 100 linii. Słysząc to, prawdopodobnie masz ochotę powiedzieć mi o pojedynczych obowiązkach i zachęcić mnie do refaktoryzacji. Jest to również mój instynkt jelitowy, ale tutaj jest problem: funkcja robi jedną rzecz. Wykonuje złożoną manipulację ciągiem znaków, a ciało funkcji składa się głównie z jednego wyrażenia regularnego, podzielonego na wiele udokumentowanych wierszy. Gdybym podzielił wyrażenie regularne na wiele funkcji, czuję, że faktycznie straciłbym czytelność, ponieważ skutecznie zmieniam języki i nie będę w stanie skorzystać z niektórych funkcji oferowanych przez wyrażenia regularne . Oto moje pytanie:

Czy w przypadku manipulacji ciągami za pomocą wyrażeń regularnych ciała o dużych funkcjach nadal stanowią anty-wzorzec? Wygląda na to, że nazwane grupy przechwytywania służą bardzo podobnie do funkcji. Nawiasem mówiąc, mam testy dla każdego przepływu przez wyrażenie regularne.

To, co napotykasz, to dysonans poznawczy, który pochodzi ze słuchania ludzi, którzy preferują niewolnicze przestrzeganie wytycznych pod przykrywką „najlepszych praktyk” zamiast uzasadnionego procesu decyzyjnego.

Wyraźnie odrobiłeś lekcje:

• Cel funkcji jest zrozumiany.
• Działanie jego implementacji jest zrozumiałe (tzn. Czytelne).
• Istnieją pełne testy wdrożenia.
• Te testy są zaliczane, co oznacza, że ​​uważasz, że implementacja jest poprawna.

Jeśli którykolwiek z tych punktów nie byłby prawdą, jako pierwszy powiedziałbym, że twoja funkcja wymaga pracy. Jest więc jeden głos za pozostawieniem kodu bez zmian.

Drugi głos pochodzi z analizy twoich opcji i tego, co zyskujesz (i tracisz) z każdego z nich:

• Refaktor. Zapewnia to zgodność z czyimś pojęciem, jak długo powinna być funkcja, i zmniejsza czytelność.
• Nic nie robić. Utrzymuje to istniejącą czytelność i poświęca zgodność z czyimś wyobrażeniem o tym, jak długo powinna trwać funkcja.

Ta decyzja sprowadza się do tego, że bardziej cenisz: czytelność lub długość. Wpadam do obozu, który uważa, że ​​długość jest dobra, ale czytelność jest ważna, a ten drugi przejmie w ciągu tygodnia.

Podsumowując: jeśli nie jest zepsuty, nie naprawiaj go.

Szczerze mówiąc, twoja funkcja może „zrobić jedną rzecz”, ale jak sam powiedziałeś

Mógłbym zacząć rozbijać wyrażenie regularne na wiele funkcji,

co oznacza, że ​​Twój kod ex ex robi wiele rzeczy. I myślę, że można go podzielić na mniejsze, indywidualnie testowane jednostki. Jednak jeśli jest to dobry pomysł, nie jest łatwo odpowiedzieć (zwłaszcza bez zapoznania się z rzeczywistym kodem). Prawidłowa odpowiedź może nie być ani „tak”, ani „nie”, ale „jeszcze nie, ale następnym razem musisz coś zmienić w tym reg. Exp”.

ale czuję, że w ten sposób straciłbym czytelność, ponieważ skutecznie zmieniam języki

I to jest sedno - masz fragment kodu napisany w języku reg ex . Ten język sam w sobie nie zapewnia dobrych metod abstrakcji (i nie uważam, że „nazwane grupy przechwytywania” zastępują funkcje). Tak więc refaktoryzacja „w języku reg ex” nie jest tak naprawdę możliwa, a przeplatanie mniejszych regów z językiem hosta może w rzeczywistości nie poprawić czytelności (przynajmniej tak uważasz , ale masz wątpliwości, w przeciwnym razie nie opublikowałbyś pytania) . Oto moja rada

• pokaż swój kod innemu zaawansowanemu programistowi (może na /codereview// ), aby upewnić się, że inni myślą o czytelności tak, jak Ty. Bądź otwarty na pomysł, że inni mogą nie znaleźć zapisu 100 linii tak czytelnego jak ty. Czasami pojęcie „jego niełatwo jest rozbić na mniejsze kawałki” można pokonać tylko przez drugą parę oczu.

• obserwować faktyczną ewolucję - czy Twoja błyszcząca rejestracja nadal wygląda tak dobrze, gdy pojawiają się nowe wymagania i musisz je wdrożyć i przetestować? Tak długo, jak twój reg exp działa, nie dotykam go, ale za każdym razem, gdy coś musi zostać zmienione, zastanowię się ponownie, czy naprawdę dobrym pomysłem było umieszczenie wszystkiego w tym jednym wielkim bloku - i (poważnie!) Przemyślenie, jeśli podzielisz się na mniejsze kawałki nie byłyby lepszym rozwiązaniem.

• obserwuj łatwość konserwacji - czy potrafisz bardzo dobrze debugować rejestr exp w obecnej formie? Zwłaszcza po tym, jak musisz coś zmienić, a teraz testy mówią ci, że coś jest nie tak, czy masz debuger reg exp pomagający znaleźć podstawową przyczynę? Jeśli debugowanie stanie się trudne, będzie to również okazja do ponownego rozważenia projektu.

Czasami dłuższa funkcja, która wykonuje jedną rzecz, jest najbardziej odpowiednim sposobem obsługi jednostki pracy. Możesz łatwo przejść do bardzo długich funkcji, kiedy zaczynasz zajmować się zapytaniami do bazy danych (używając swojego ulubionego języka zapytań). Uczynienie funkcji (lub metody) bardziej czytelną przy jednoczesnym ograniczeniu jej do określonego celu uważam za najbardziej pożądany wynik funkcji.

Długość jest arbitralnym „standardem”, jeśli chodzi o rozmiar kodu. Tam, gdzie funkcja 100 linii w języku C # może być uważana za długotrwałą, w niektórych wersjach zestawu byłaby niewielka. Widziałem niektóre zapytania SQL, które były dobrze w 200 wierszach zakresu kodu, które zwróciły jeden bardzo skomplikowany zestaw danych do raportu.

Celem jest w pełni działający kod , który jest tak prosty, jak tylko można rozsądnie .

Nie zmieniaj go tylko dlatego, że jest długi.

Zawsze możesz podzielić wyrażenie regularne na wyrażenia regularne i stopniowo komponować ostateczne wyrażenie. Może to pomóc w zrozumieniu bardzo dużego wzorca, szczególnie jeśli ten sam wzorzec jest powtarzany wiele razy. Na przykład w Perlu;

my $start_re = qr/(?:\w+\.\w+)/;
my $middle_re = qr/(?:DOG)|(?:CAT)/;
my $end_re = qr/ => \d+/;

my $final_re = $start_re . $middle_re . $end_re;
# or: 
# my $final_re = qr/${start_re}${middle_re}${end_re}/

Powiedziałbym „zepsuć”, jeśli jest to łamliwe. z punktu widzenia łatwości utrzymania i być może sensowności jest sensowne, aby je złamać, ale oczywiście musisz wziąć pod uwagę naturalność swojej funkcji oraz sposób, w jaki otrzymujesz dane wejściowe i to, co ma zamiar zwrócić.

Pamiętam, że pracowałem nad analizowaniem strumieniowania podzielonych danych na obiekty, więc po prostu podzieliłem je na dwie główne części, jedną budowałem kompletną jednostkę String z zakodowanego tekstu, a drugą analizowałem te jednostki w słowniku danych i organizowałem je (może być losową właściwością dla innego obiektu) i niż aktualizowanie lub tworzenie obiektów.

Mogłem również podzielić każdą główną część na kilka mniejszych i bardziej specyficznych funkcji, więc na koniec miałem 5 różnych funkcji do zrobienia wszystkiego i mogłem ponownie użyć niektórych funkcji w innym miejscu.

Jedną z rzeczy, które mogłeś wziąć pod uwagę lub nie, jest napisanie małego parsera w języku, którego używasz, zamiast używania wyrażenia regularnego w tym języku. Może to być łatwiejsze do odczytania, przetestowania i utrzymania.

Olbrzymie wyrażenia regularne są w większości przypadków złym wyborem. Z mojego doświadczenia wynika, że ​​są one często używane, ponieważ programista nie zna parsowania (zobacz odpowiedź Thomasa Edinga ).

W każdym razie załóżmy, że chcesz trzymać się rozwiązania opartego na wyrażeniach regularnych.

Ponieważ nie znam faktycznego kodu, zbadam dwa możliwe scenariusze:

• Wyrażenie regularne jest proste (dużo dosłownego dopasowania i kilka alternatyw)

  W tym przypadku zaawansowane funkcje oferowane przez pojedynczy regex nie są niezbędne. Oznacza to, że prawdopodobnie skorzystasz z podziału.

• Wyrażenie regularne jest złożone (wiele alternatyw)

  W takim przypadku nie możesz realistycznie mieć pełnego zasięgu testu, ponieważ prawdopodobnie masz miliony możliwych przepływów. Aby go przetestować, musisz go podzielić.

Mogę brakować wyobraźni, ale nie mogę sobie wyobrazić żadnej sytuacji w świecie rzeczywistym, w której regex 100-liniowy jest dobrym rozwiązaniem.