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

15

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.

DudeOnRock
źródło
3
Nie sądzę, aby coś było nie tak z twoją funkcją, biorąc pod uwagę, że duża jej część to dokumentacja . Może jednak wystąpić problem z utrzymywaniem przy użyciu dużego wyrażenia regularnego.
Joel Cornett,
2
Czy jesteś pewien, że wielkie wyrażenie regularne jest najlepszym rozwiązaniem Twojego problemu? Czy zastanawiałeś się nad prostszymi alternatywami, takimi jak biblioteka analizatora składni lub zastąpienie niestandardowego formatu pliku standardowym (XML, JSON itp.)?
lortabac
2
Czy istnieją inne funkcje korzystające ze zmienionej / ulepszonej / uproszczonej wersji tego wyrażenia regularnego? Byłby to ważny wskaźnik, że należy dokonać refaktoryzacji. Jeśli nie, zostawiłbym to tak, jak jest. Potrzebowanie takiej złożonej manipulacji ciągiem to sama w sobie żółta flaga (nie znam kontekstu, a więc tylko żółta), a refaktoryzacja funkcji wydaje mi się bardziej rytuałem wykupienia poczucia winy, o którym się myśli it;)
Konrad Morawski
8
W jaki sposób wyrażenie regularne 100 może zrobić tylko jedną rzecz?
Pieter B
@lortabac: Dane wejściowe to tekst generowany przez użytkownika (proza.)
DudeOnRock

Odpowiedzi:

36

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.

Blrfl
źródło
10
+1 dla „Jeśli nie jest uszkodzony, nie naprawiaj go”.
Giorgio
W rzeczy samej. Zasady Sandy Metz ( gist.github.com/henrik/4509394 ) są fajne i wszystkie, ale na youtube.com/watch?v=VO-NvnZfMA4#t=1379 opowiada o tym, jak powstały i dlaczego ludzie biorą o wiele za poważnie.
Amadan
@Amdan: Z dodatkowym kontekstem filmu wideo to, co zrobił Metz, ma sens. Jej zalecenie dla tego jednego klienta było celowo ekstremalne z jednej strony, aby przeciwdziałać ekstremalnym zachowaniom z drugiej strony, jako sposób na przeciągnięcie go do bardziej rozsądnego środka. Reszta tej dyskusji sprowadza się do sedna mojej odpowiedzi: rozumowanie, a nie wiara, jest sposobem na określenie najlepszego sposobu działania.
Blrfl,
19

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.

Doktor Brown
źródło
Powiedziałbym, że nazwane grupy przechwytywania (ogólnie grupy przechwytywania, naprawdę) są najbardziej podobne do zmiennych końcowych / jednorazowych do zapisu, a może makr. Pozwalają odwoływać się do określonych części dopasowania, albo z obiektu dopasowania zwróconego z procesora wyrażenia regularnego, albo później w samym wyrażeniu regularnym.
JAB
4

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.

Adam Zuckerman
źródło
3

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}/
Rory Hunter
źródło
Używam pełnej flagi, która jest jeszcze wygodniejsza niż to, co sugerujesz.
DudeOnRock
1

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.

arfo
źródło
1

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.

Thomas Eding
źródło
Sam o tym myślałem. Problem polega na tym, że wkładem jest proza, a ja czerpię wskazówki z kontekstu i formatowania. Jeśli jest możliwe napisanie parsera dla czegoś takiego, chciałbym dowiedzieć się więcej na ten temat! Sam nic nie mogłem znaleźć.
DudeOnRock
1
Jeśli wyrażenie regularne może je przeanalizować, możesz je przeanalizować. Twoja odpowiedź sprawia, że ​​wydaje mi się, że możesz nie być dobrze zorientowany w analizie składni. W takim przypadku możesz chcieć trzymać się wyrażenia regularnego. Albo to, albo naucz się nowych umiejętności.
Thomas Eding,
Chciałbym nauczyć się nowych umiejętności. Jakieś dobre zasoby, które możesz zasugerować? Interesuje mnie teoria, która się za tym kryje.
DudeOnRock
1

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.

lortabak
źródło