Jakość kodu w waszych projektach/w waszej pracy

W mojej obecnej pracy "kod wygląda jak kod" tzn. często trzeba go przeczytać, żeby widzieć co robi (nazwy metod niekoniecznie wszystko przekazują). Ale generalnie wiadomo gdzie czego szukać, jak trzeba dodać jakąś funkcjonalność, czy coś sfixowac. Ciężko jest znaleźć nieprzetestowany kod (testami automatycznymi, niekoniecznie jednostkowymi). A projekt to około 1,5 mln LOC.

Dobra, do tego kod jest otestowany i przechodzi code review

U mnie pół na pół. Mamy kilka projektów z kodem dobrej jakości z ładnymi testami, ale jest też kilka potworków, których chętnie byśmy się pozbyli gdyby się dało ;] Na szczęście to są rzeczy oznaczone jako deprecated i nie ma mowy o jakimkowiek developmencie, co najwyżej raz na jakiś czas trzeba gasić pożary i łatać bugi. Niemniej tam można zobaczyć kod z serii what has been seen cannot be unseen ;]

@scibi92 z tym "brzydkim" kodem u ciebie w pracy, szczególnie do seniorów to bym uważał. Bo juniorzy często mają takie mylne wyobrażenie że "to jest bez sensu, zrobiłbym lepiej" a jak się zagłębisz w problem, przypadki brzegowe i różne dziwne wymagania to nagle się okazuje że zrobiłbyś tak samo, albo jeszcze gorzej by to wyglądało.

@Shalom
Ja mam brak seperacji na poszczególne składowe albo klasy na 1000 linijek ze static Stringami ;]

U mnie jest średnio, miałem projekty z lepszym kodem, ale np ostatni był tak przekombinowany (kilka wzorców na raz+convention over configuration), że wolę ten gorszy kod, bo potrafię szybko go zdebugować, naprawić. Inna sprawa, że wcześniej to był produkt, a teraz mam kod który działa 24h na produkcji i sam muszę go utrzymywać.

Jeśli jest jakiś mega zły kawałek kodu i inne osoby z zespołu również widzą w nim wady to po prostu jest to dług technologiczny. Jeśli się da to trzeba po kawałku spłacać.
Ja się nikogo nie pytam czy mogę coś zrefaktoryzować albo przepisać, po prostu tak estymuje taski, by systematycznie i po fragmencie polepszać kod. Nie jest to na początku łatwe, ale dużo osób tak robi i jest skuteczne.

Jeśli syf jest w prawie każdej klasie, wszelkie zmiany ciągną się tygodniami, na propozycje testów automatycznych wszyscy są na nie, to lepiej podziękować za współpracę. Są osoby które nigdy nie napiszą dobrego kodu dopóki nie zmienią myślenia/zespołu/firmy i praca z nimi jest ciężka.

Spotkałem też osoby które miały przegięcie w drugą stronę tzn mega pedantyczne i IMHO jest to równie męczące co skrajne druciarstwo.

Nie stosuję żadnych konwencji. Ważne jak działa program a nie czy zmienna nazywa się tak czy śmak czy duże czy małe litery albo gdzie wcięcie.

A najnowsze kompilatory obsługują w końcu polskie litery w nazwach zmiennych czy klas. W moim kodzie zawsze wszystko jest po polsku z polskimi znakami w nazwach - Polacy nie gęsi swój język mają.

Aha i nie marnuję czasu na jakieś głupawe testy - trzeba umieć myśleć w trakcie i pisać kod bez błędów. Jak ktoś nic nie umie to potem musi testy pisać, żeby sprawdzić sam siebie czy dobrze zrobił. Śmieję się z takich bo to znaczy że nic nie umie

U mnie w pracy przykłada się dużą wagę do jakości kodu. Po każdym commicie startuje statyczna analiza kodu i parę narzędzi badających czy kod nie łamie ustalonych konwencji i zasad, więc jakość jest całkiem całkiem.

Minusem jest to, że ze wzgledu na dość skomplikowaną domenę, kod z logiką biznesową jest po polsku (żeby uniknąć nieporozumień przy tłumaczeniach). Na początku wygląda to kosmicznie, szczególnie w miejscach gdzie logika w jakimś stopniu miesza się z kodem obcych bibliotek pisanych oczywiście po angielsku ;-)

Weźcie pod uwagę to że to co dla Was jest syfem, dla kogoś innego będzie cool. No i weźcie pod uwagę także sam model biznesowy firm w których pracujecie. I frameworki (własne rozwiązania vs. popularne rozwiązania). Tak np. kod jakiegoś popularnego CMS-a będzie syfem, zaś kod tej samej aplikacji na frameworku będzie ładny i wszystko łatwe do utrzymania. Jest tylko pytanie czy ta firma która używa FW będzie konkurencyjna vs. ta która używa CMS-y.

Możliwe że ten syf jest także celowy, bo przecież wiadomo że jak Wam ktoś coś zleci to nie chcielibyście żeby potem ktoś tańszy od Was to dalej rozwijał tylko lepiej żeby klient to Wam powierzył, za większą kasę bo nie będzie chętnych do rozwoju tego kodu.

Kiedy to wszedłem na jedno coraz bardziej popularne już forum informatyczne które stoi na platformie Question And Answer w PHP i śmiałem tylko wspomnieć o Laravelu albo Kohanie to zaraz w komentarzach posypało się kilka komentarzy (nie pochwalających tych rozwiązań), jak również otrzymałem kilka downvote :-D

Zastanawiam się tylko czy niektórzy aż za bardzo nie przesadzają bo wyglądałoby to raczej na ewangelizację niż to ile to może wnieść faktycznie :-) Za rok, może dwa to może będzie coś lepszego od Symfony albo będą w nim nowe rozwiązania a te stare będą opatrzone niepochlebnymi opiniami. Tak np. Active Record o którym śmiałem wspomnieć vs. Data Mapper i jakoby Active Record był przestarzały. Zastanawiam się tylko jakie to ma znaczenie w przypadku małej aplikacji a forum na które wtedy wszedłem na większy projekt nie wyglądało.

Shalom napisał(a)

Bo juniorzy często mają takie mylne wyobrażenie że "to jest bez sensu, zrobiłbym lepiej" a jak się zagłębisz w problem, przypadki brzegowe i różne dziwne wymagania to nagle się okazuje że zrobiłbyś tak samo, albo jeszcze gorzej by to wyglądało.

Czasem tak, czasem nie.

szczególnie do seniorów to bym uważał.

Czemu "szczególnie do seniorów"? Widziałem już kody tworzone przez podobno "seniorów", które były na poziomie "junior programmer". Jak dla mnie senior to bardziej pozycja w hierarchii niż miernik skillów. Senior może mieć lepsze skille niż junior, czy regular, ale nie musi.

sarin napisał(a)

Spotkałem też osoby które miały przegięcie w drugą stronę tzn mega pedantyczne i IMHO jest to równie męczące co skrajne druciarstwo.

Ano, można przesadzić w drugą stronę. Pracowałem kiedyś z kimś, kto naprawdę miał super skille, ale miał fioła na punkcie jakości kodu (z formatowaniem włącznie) i każdy pull request przechodził przez code review z bólem i tysiącami mini-poprawek typu "zlikwidować spacje po średniku w linii 89" (to teoretycznie powinno wychodzić w narzędziu do statycznej analizy kodu, ale jednak nie wszystkie zasady były tam wpisane, więc i tak na code review to wyłaziło).

Może od strony programistycznej/edukacyjnej jest to fajne (sam nauczyłem się doceniać jakość kodu wtedy), ale jednak od strony biznesowej nie za bardzo, bo zamiast zmerdżować dzisiaj coś w 85% dobre, to się dopieszcza to do 99% tracąc mnóstwo czasu na poprawki i generując opóźnienia w sprincie.

Nie mówię, że nie warto dbać o jakość kodu, tylko raczej, że warto wiedzieć, kiedy przestać :)

No ale w drugą stronę idąc.

Pracowałem kiedyś z kodem, który był strasznej jakości i miał pełno kopiuj-wklejek.

I co? No i wciągnęło mnie to. Zacząłem wszystko refaktoryzować do bólu i tworzyć moduły do ponownego użycia. Tyle, że wygenerowałem przez to (między innymi) opóźnienia w projekcie, przez co zebrałem opieprz (chociaż moim zdaniem refaktor był naprawdę potrzebny w tym przypadku, bo to było zwyczajne spłacenie długu technologicznego zaciągniętego przez ludzi, którzy rozwijali ten projekt przed nami).

Ale mimo wszystko kopiuj-wklejki są o wiele lepsze i tak niż spaghetti kod albo niż kod przeinżynierowany(tzw. ravioli code: http://c2.com/cgi/wiki?RavioliCode ), ponieważ kopiuj wklejki są łatwe do ogarnięcia co się dzieje w tym kodzie, i do zrefaktoryzowania. O spaghetti kodzie czy kodzie przeinzynierowanym ciężko już to powiedzieć (zresztą sam teraz stosuję kopiuj-wklejki na początkowym etapie, a dopiero potem wydzielam sensowne moduły, trochę zainspirowawszy się tym: http://www.sandimetz.com/blog/2016/1/20/the-wrong-abstraction . Chociaż kiedy czytałem ten artykuł to shejtowałem go, dopiero później mnie tknęło, że coś w tym jest.

Czemu "szczególnie do seniorów"?

Senior seniorowi nie równy ale jednak jak ktoś ma doświadczenie to spada szansa że pisze zły kod bo nie umie.

ponieważ kopiuj wklejki są łatwe do ogarnięcia co się dzieje w tym kodzie, i do zrefaktoryzowania

Żebyś się nie zdziwił. Copypaste ma to do siebie że często te fragmenty są lekko zmienione, mają jakieś własne przypadki brzegowe i trudno te subtelne różnie wyłapać, w efekcie refator to pole minowe bo może ci się wydawać że dobrze wydzieliłeś wspólny fragment a wcale tak nie jest.

zresztą sam teraz stosuję kopiuj-wklejki na początkowym etapie, a dopiero potem wydzielam sensowne moduły

I pewnie kończysz z połową kodu z duplikacjami bo zwyczajnie zapomniałeś że tu i tam coś było skopiowane. Powodzenia z takim podejsciem :D
A artykuł dość tendencyjny, bo problem opisywany przez autora wynikał nie z wprowadzonej abstrakcji tylko z tego że kolejni developerzy źle ja rozwijali przez dodawanie parametrów i robienie metody "z wajchą" (tzn z parametrem który sprawia że metoda robi coś zupełnie innego).

Copypaste ma to do siebie że często te fragmenty są lekko zmienione,

Mnie tego nie musisz mówić. Znam ten ból i naprawdę mnóstwo czasu spędziłem wtedy na refaktoringu z powodu właśnie wyłapywania różnic i podobieństw. Ale i tak uważam, że lepsze to niż spaghetti kod, w którym się wszystko przeplata za wszystkim. Tak zwane "mniejsze zło" :)

trudno te subtelne różnie wyłapać,

To zależy. Myślę, że można to zautomatyzować. Eksperymentowałem z pisaniem automatycznego deduplikatora do kodu HTML i nawet mi się udało zrobić proof of concept, który robił ze zduplikowanego kodu HTML komponenty React i sam wydzielał dla nich odpowiednie parametry analizując co jest stałe, a co jest różne w każdym fragmencie.

Więc jest to możliwe. Co prawda z HTMLem jest o tyle łatwo, że ma on prostą hierarchiczną strukturę i łatwo go analizować statycznie. W przypadku języka programowania mogłoby być trudniej (chociaż nie jest to niemożliwe - w IntelliJ jest opcja replace duplicates, która ponoć automatycznie to wykrywa i pozwala zamieniać (niestety w WebStormie, z którego korzystałem nie ma takiej opcji z tego co wiem :( ).

W każdym razie myślę, że tutaj jeszcze statyczna analiza kodu oparta na AST (tudzież automatyczny refaktoring) ma tutaj duże pole do popisu.

A artykuł dość tendencyjny

Wiem że jest tendencyjny, dlatego jak mówię go shejtowałem z tych samych powodów o których piszesz. Cytując samego siebie:
: I often see copy pasted code which have these same characteristics author wrote about: "Another additional parameter. Another new conditional. Loopuntil code becomes incomprehensible." If somebody writes too many conditionals, parameters and creates incomprehensible loops, then he will do it always, no matter whether in abstracted code or in copy pasted code...
https://news.ycombinator.com/item?id=11047054

Bo te przykłady faktycznie były trochę z d**y.

Ale po przemyśleniu, to trochę w tym prawdy jednak jest, że lepsze czasem efekty daje zduplikowanie czegoś na początku, a potem wydzielenie modułu, niż wydzielenie błędnych abstrakcji a potem męczenie się z nimi.

I pewnie kończysz z połową kodu z duplikacjami bo zwyczajnie zapomniałeś że tu i tam coś było skopiowane. Powodzenia z takim podejsciem

raczej miałem na myśli kontrolowaną duplikację na małą skalę. Poza tym robię to tylko pracując w pojedynkę i mając pod kontrolą wszystko co się dzieje w projekcie. Jeśli robię projekt w więcej osób to jestem przeciwko takim praktykom, ponieważ w projektach wieloosobowych, gdzie pracuje się od tasku do tasku jak ktoś coś źle napisze, to potem tak zostanie na amen i dopiero następne osoby po iluś miesiącach będą spłacać ten dług techniczny (ja też przecież musiałem spłacać dług techniczny za te osoby, które radośnie kopiowały sobie wszystko).

zapomniałeś że tu i tam coś było skopiowane.

do tego służą komentarze // TODO, żeby o tym nie zapomnieć. Poza tym kopiuj wklejki w pewnych miejscach są zupełnie nieszkodliwe i łatwe do refaktoryzacji. Trzeba wiedzieć tylko w których.

Problem tylko, że większość programistów ma za mało doświadczenia, żeby odróżnić nieszkodliwe kopiuj wklejki od szkodliwych.

I drugi problem, że pracując w teamie w projekcie rozwijanym na przestrzeni kilkanastu miesięcy zwykle i tak o wszystkim się zapomni, a TODO będą leżały miesiącami, aż połowa programistów odejdzie, a przyjdą nowi i nawet jak zobaczą TODO, to zwykle i tak z nim nic nie zrobią, tylko będą dodawać kolejne duplikacje.

Myślę, że większą szkodą dla projektów są ludzie i ich proces pracy niż sam zduplikowany kod.

Więc generalnie nie polecam tego na większą skalę jak się rozwija długotrwały projekt w ileś osób. Co innego jeśli się robi coś samemu, albo w małym zespole i będzie się w stanie samemu zrobić potem porządek z kodem (albo udokumentować w jasny sposób, że coś jest duplikacją).