fbpx
devstyle.pl - Blog dla każdego programisty
devstyle.pl - Blog dla każdego programisty
7 minut

Nie krytykuj


25.02.2013

Przychodzi baba do lekarza. Mówi “panie doktorze, byłam już u doktora X i on zalecił mi Y“. Na co lekarz: “a-HA! znam X, to konował, nic nie umie, tak naprawdę to dopiero JA pani powiem co trzeba zrobić“. Baba jest od tej pory zakochana w swoim nowym doktorze, nie mając tak naprawdę pojęcia czy faktycznie jest tak dobry jak twierdzi. Chodzi po sąsiadach i rozpowiada jaki to on jest cudowny, bo z taką pewnością pojechał po X, że coś musi w tym być. A do X to ona już nigdy nie pójdzie skoro jest taki słaby.

Przychodzi klient do programisty. Mówi “panie programisto, byłem już i programisty X i on mi napisał ten kod“. Na co programista: “ojezuu, takiego ścierwa w życiu nie widziałem, to się nadaje tylko do kosza, nic z tym się nie da zrobić, wyrzuć i napisz od nowa“. Zdarzy się (mi się zdarzyło gdy “byłem jeszcze taki”), że ów klient zachowa się podobnie jak wspomniana wcześniej baba – zakocha się w tak pewnym siebie i najwidoczniej zakochanym w jakości programiście, psy wieszającym na poprzedniku. “Och, drogi nowy programisto, jakież to nieszczęście że dopiero teraz ścieżki nasze się krzyżują, od tej pory zlecę ci wszystkie moje nowe projekty“. Domyślić się można, że te nowe projekty, pokazane jeszcze innemu programiście, mogą zostać potraktowane w identyczny sposób jak oryginalny kod programisty X.

Nie wiem jak wam, ale mi taki scenariusz nie jest obcy (w obu przypadkach). Co ta sytuacja pokazuje? Że bardzo łatwo jest zbudować sobie reputację alfy i omegi jadąc bez trzymanki po czyjejś pracy jak po burej suce, najlepiej z uśmieszkiem pełgającym w kąciku ust. Zerowym praktycznie wysiłkiem. Czy to faktycznie świadczy o wielkim doświadczeniu, o nieomylności i genialności “krytyka”? Nie. Do każdego kodu można się przyczepić. Nie ma kodu, którego nie da się napisać lepiej. Skrytykowanie cudzego kodu to zadanie najłatwiejsze na świecie dla każdego programisty, i szczególnie ci “dojrzewający” lubują się w podobnym gilganiu własnego ego. “To jest napisane bez DDD? Co za shit!“. “Nie ma ani jednej klasy wskazującej na zastosowanie wzorców projektowych? Jaki ciołek to pisał?“. Itd…

Porada “system jest tak źle napisany, że trzeba go napisać od nowa” to najbardziej amatorskie i godne pożałowania podejście, jakie można zaprezentować. Nie ma gwarancji, że po X miesiącach pracy nie skończymy z taką samą kupą kodu, zawierającą te same lub inne niedociągnięcia. A właściwie jest gwarancja, że tak się właśnie stanie. W Rapid Development fajnie opisano takie historie.

Bo niby czemu miałoby być inaczej? Projektowanie i implementacja systemu zajmuje czas. W tym czasie stajemy się (miejmy nadzieję) coraz lepsi w swoim fachu. Efekt: po zakończeniu projektu moglibyśmy go napisać lepiej. Ja przyznaję się bez bicia: każdy napisany przeze mnie projekt ssie. Dlaczego? Bo dziś napisałbym go lepiej niż rok temu czy pięć lat temu. I to jest DOBRY znak. To znak, że nie stoję w miejscu. Ale to także znak, że projekt, który zaimplementuję dzisiaj wspinając się na wyżyny swoich programistycznych, za rok nie będzie wyznacznikiem mojej top-formy. Bo za rok napisałbym go lepiej.

Nie ma kodu idealnego. Jest co najwyżej kod wystarczająco dobry. Ale nie oszukujmy się: większość kodu, z jakim przychodzi nam się na co dzień stykać, nie jest nawet taka. I to się nigdy nie zmieni.

Zdarzało mi się dostawać prośby o przejęcie lub review cudzego kodu.
Najbardziej zapadający w pamięć przypadek to księgarnia online. Dostałem dostęp do repozytorium z pytaniem “czy da się to dalej rozwijać?“. I jeśli tak – to czy bym się tego podjął. Była to aplikacja napisana w MVC, z tym że… 90% kodu znajdowała się w jednej statycznej klasie: Manager. Ten Manager robił wszystko: od dostępu do danych, przez wyliczanie rabatów, po zarządzanie całym procesem składania i obsługi zamówienia. Pierwsza moja myśl, jak już się zorientowałem o co tam chodzi, brzmiała, jak się można domyślić: “o kur…”. Mogłem potencjalnemu klientowi odpisać, że to najgorszy kod jaki w życiu widziałem. Że nie da się z tym nic zrobić. Że chętnie napiszę to od nowa (licząc oczywiście stawkę x2 czy x3 bo terminy były już bardzo naglące). Że jestem tak doświadczony i zajebisty że mnie całuj po stopach, a napiszę to lepiej.

Ale tego nie zrobiłem. Dlaczego? Bo to DZIAŁAŁO. Jak mogę zarekomendować wyrzucenie do kosza czegoś, co działa? Odpisałem, że jeśli system funkcjonuje jak powinien i są z niego zadowoleni, to w chwili obecnej (terminy) najlepiej będzie brnąć w niego dalej. Ale że ja się tego nie podejmę. I że wprowadzenie zmian w przyszłości będzie bardzo niebagatelnym i kosztownym procesem. Jedyne co mogę zrobić to zaproponować pewne konkretne rozwiązania w określonych obszarach, ale to samo w sobie jest osobnym zleceniem, którego efektem nie będzie nowa funkcjonalność ani lepszy kod, a jedynie wydeptanie ścieżki którą przyszły programista powinien podążać.

Sztuką jest wyodrębnienie w analizowanym systemie pewnej rodziny problemów i zaproponowanie ich rozwiązania, wraz z korzyściami, jakie ono przyniesie. Wyciągnięcie wniosków z cudzych (lub swoich) błędów i upewnienie się, że sami ich więcej nie popełnimy. Zaplanowanie refactoringu mającego na celu eliminację najbardziej palących kwestii.

I to właśnie rozumiem przez “code review” bądź “audyt”. Owszem, tracimy w ten sposób satysfakcję płynącą z mieszania czyjejś pracy z błotem i rośnięcia w oczach słuchacza/klienta, który być może nie ma zielonego pojęcia o czym mówimy i widzi w nas, jak baba z pierwszego akapitu, nowego najlepsiejszego doktora.

Konstruktywna krytyka z konkretnymi propozycjami ulepszeń jest potrzebna. Krytyka dla krytyki, bez analizy, diagnozy i wniosków – nic nie wnosi. Takiej mówimy, jak parówkowym skrytożercom, mocne i stanowcze NIE!

Mały disklejmer, aby ubiec komentarze drobiazgowych analizatorów: cały ten post traktuje o krytyce KODU z perspektywy PROGRAMISTY. Moi twitterowi śledczy mogą zauważyć jak bardzo narzekam na wszelkie oprogramowanie z którym mam styczność. To chyba z 80% moich tweetów. Ale jest pewna subtelna różnica: narzekam z perspektywy UŻYTKOWNIKA… a to zupełnie inna para kaloszy.

0 0 votes
Article Rating
16 Comments
Oldest
Newest Most Voted
Inline Feedbacks
View all comments
mihcall
mihcall
11 years ago

disclaimer na końcu najlepszy! :)

trackback
11 years ago

Nie krytykuj | Maciej Aniserowicz o programowaniu…

Dziękujemy za dodanie artykułu – Trackback z dotnetomaniak.pl…

pawelek
pawelek
11 years ago

No ale czekaj. Widzisz kod, który z daleka śmierdzi. Owszem – stawiasz się w roli piszącego, który zapewne miał powód by tak ten kod napisać ( i niekoniecznie tym powodem był brak wiedzy ). Ale mimo wszystko, jeśli jesteś coś w stanie lepiej napisać, to nie rozumiem czemu miałbyś nie powiedzieć, że Ty jednak zrobiłbyś to tak czy siak. Może Twoja opinia trafi z powrotem do gościa i on się czegoś nauczy. Ba może ktoś zrobi opinię Twego kodu i nauczysz się czegoś ciekawego? To jest krytyka właśnie więc nie rozumiem czemu nie krytykować?
Mówienie o kimś, że jest do d*** jest be już z podstaw savoir-vivre :)

orientman
11 years ago
Gutek
11 years ago

Szczerze, nie pamietam abym tak powiedzial wprost klientowi, zawsze za to wspominam, ze moze byc problem z rozszerzalnoscia i czas pracy mojej moze byc wydluzony gdyz cos cos cos. Czasami takze proponuje poprawienie tego z mysla o tym, ze dany projekt moze sie rozrosnac, wiec wspominam, ze aktualnie bedzie ql i to dziala, ale jak w przyszlosci bedziecie chcieli na przyklad cos dodac lub zrobic kolejna wersje z czyms tam, to natraficie na problem (czasowy jak i (nie)wykonywalny).

ale by tak jechac do klienta na temat dev ktory to pisal? moze to nasz kumpel ;) to wtedy jemu sie na priv pojedzie :)

Arek Bal
Arek Bal
11 years ago

A gdyby tak zamiast klientowi mówić że refaktor będzie drogi to zaprezentować alternatywę… czyli że nie musi być aż tak drogi jeżeli klient zgadza się na ryzyko utraty części funkcjonalności. Wyznaczamy kluczowe dla niego funkcjonalności które muszą zostać, a resztę wycinamy/refactorujemy ostro i na bieżąco ożywiamy. Część tych funkcjonalności może się okazać, że wcale nie musi ożywać i wszyscy będą bardziej zadowoleni.

Gutek
11 years ago

Procent,

Tym bardziej mnie dziwi, ze takie cos ma miejsce :(

Wojtek
Wojtek
11 years ago

Code-review to z jednej strony ważny temat, ale z drugiej strony również dosyć delikatny. Prowadzony nieumuejętnie szybko zepsuje atmosferę w zespole. Wiadome, nikt nie lubi być krytykowany, każdy ma swoje ambicje, swoje ego, swoją wartość i każdy chce być najlepszy na świecie. Zarówno krytykowania jak i przyjmowania krytki trzeba się nauczyć. Dołączyłem kiedyś do zespołu, w którym leader wykonywał code-review po każdym commicie i wysyłał maila do wszystkich z komentarzami i uwagami, co pasowało by poprawić, co jest ryzykowne, na co zwrócić uwagę. Po pierwszych commitach byłem totalnie sfrustrowany, ale też wyciągałem wnioski i z czasem maili było coraz mniej.

Po pewnym czasie review zaczął pisać też jeden z kolegów z zespołu (nasz rówieśnik). Owszem był dobry, jego uwagi były trafne, ale sposób i język jakim je pisał okazywaly wyraźną pogardę dla programisty, którego kod był komentowany. I to niestety nie było dobre. Ja już nie pracuję dla tamtej firmy (z innych przyczyn), ale wiem, że kolega komentator nie ma zbyt wielu znajomych w zespole, a niektórzy się do niego nie odzywają.

Zatem wniosek jaki mi się nasuwa to: język. Język w jakim krytykujemy jest mega ważny bo ma motywować, a nie gnoić. Zamiast rozkazywać, może lepiej zachęcić do poprawy. I zamiast tylko pokazywać błędy, warto też proponować ich rozwiązania.

Jest jeszcze kwestia hierarchii w zespole. Nawet najgorszą krytykę lepiej się przyjmuje od przełożonego (czy leadera), niż od kolegi z biurka obok. No ale, nie każdy dostał tyle samo talentów – deal with it :)

Arek Bal
Arek Bal
11 years ago

To prawda Wojtku. Spotkania Scrumowe to taka esencja, bitwa na argumenty (w lepszych zespołach), lub bitwa na autrytety (w gorszych zespołach). Zawsze walczę o to by dyskusja była rzewna ale pełna argumentów, a nie jechanie po kimś. A jak się chce komus dowalić to ja znam klika takich ukochanych manipulacyjnych zwrotów które warto znac by się nie dać zdołować… oto krótki wstęp do listy:
– dlaczego się pomyliłeś? (w reakcji na przeoczenie, tzw. literówkę, nieuniknioną cześć fachu, powtarzane kilkukrotnie mimo wogóle braku istniejącej rozsądnej odpowiedzi),
– “zawsze” to coś robisz źle
– “jesteś” w tym czymś słaby

Marek
Marek
11 years ago

Coz jesli chodzi o krytykowanie osob to sie zgodze – nawet jesli mialoby pomoc to jest smieszne/zalosne. Jednak jesli idzie o kod/prace to mnie osobiscie meczy to miekkie zachodnie/indyjskie podejscie. Jesli wyniki sa zle to nalezy to wskazac i podjac temat (pomijam sytuacje gdy klient swiadomie sie decyduje na gorsze rozwiazanie). Sam mam aplikacje, ktore mialy byc stworzone w krotkim okresie czasu i jak managerowie sie o nie pytaja to mowie otwarcie ze moga ich uzyc ale sa do niczego i jesli chca czegos rozwojowego to nalezy to napisac (zazwyczaj po tym odchodza :)).

Co do historii z aplikacja klienta – myslisz ze tak samo bys odpowiedzial gdybys musial sie ta aplikacja zajac i utrzymywac ja przez najblizsze 5 lat?

Kurs Gita

Zaawansowany frontend

Szkolenie z Testów

Szkolenie z baz danych

Książka

Zobacz również