[Reevo]

Dostałem Code Review, robione przez zewnętrzną firmę, z wyszczególnionymi ~20 błędami. Moim zdaniem tylko 2 były sensowne, a reszta wręcz szkodliwa.

Próbowałem się do gościa dobić i przegadać dlaczego uważa tak, a nie inaczej, ale nie miałem okazji. Raz udał mi się wysłać mu wiadomość na discordzie, ale odpowiedział tylko że 'nie pamięta już szczegółów'.

Po 6 miesiącach nadal mnie to swędzi i czuję krytykę z tyłu głowy jak pracuję. Chyba z 50 razy już analizowałem tą listę wzdłuż i wszerz, kupiłem 3 książki na ten temat, a nadal nie jestem pewien który z nas jest upośledzony

Wiem gdzie typ pracuję, myślałem żeby wysłać mu maila z zaproszeniem do przegadania tego, ale nie chcę wyjść na świra.

Kilka dni temu wdrażałem dwa nowe pokemony do projektu (2 regularów) i dałem im im listę tych uwag w formie 'rekrutacji', żeby zobaczyć co by zrobili na moim miejscu.

Odpowiedzi były raczej w moją stronę, ale to jeszcze świeżaki, więc tym bardziej nie wiem czy to dobrze.

Panie doktorze, ego mnie boli (╥﹏╥)

#programowanie #programista15k

[aquapure]

@Reevo: ej, ale co Ci nie pasuje w tej uwadze o odwroceniu zaleznosci? Bo ja sie z tym zgadzam

[CyprianFonda]

@Reevo: yhym

[aquapure]

@Krolik: no architektura i zalozenia nie sa zero jedynkowe, ze albo wszystko piszemy rozszerzalne albo wszystko piszemy "z palca i na zawsze" i nie ma nic pomiedzy.

To kwestia wywazenia, wyczucia domeny, intuicji nabytej z doswiadczeniem, potrzeb klienta, umiejetnosci, zespolu, zasobow czasu i wielu, wielu innych kwestii.

Nie ma uniwersalnych odpowiedzi.

[henen]

@Reevo: kod było puścić przez sonarqube, jak są takie cwaniaki niech na bugach/code smells z nimi dyskutują

[314159]

@Reevo:

Mam zadeklarowaną klasę 'AppColors' z listą kolorów używanych w aplikacji jako stałe statyczne (const static).

Feedback mówi, że AppColors powinno być abstrakcyjnym interfejsem i implementacją wstrzykniętą jako globalny singleton przed DI, żeby zachować odwrócenie zależności i programowanie pod interfejs.

ale to wygląda jak downgrade. p-----l typa, nie zawsze to, co w review jest lepsze. singleton w tym przypadku nie jest absolutnie potrzebny, a same singletony nie cieszą się dobrą

[XanderPLXE]

@Reevo: Kiedy termin jest na wczoraj, bo się pali, to wszelakie zasady często idą w p---u.
Jak mi na studiach doktor powiedział jak oddawałem program: "dobrze, ale ja bym zrobił inaczej. 95/100 pkt"

[croppz]

singletony nie cieszą się dobrą sławą przecież, coś jak antypattern

@314159: ludzie marudzą bo gdzieś tak usłyszeli i bezmyślnie powtarzają, jednocześnie korzystają i często nawet nie zdają sobie z tego sprawy. ¯\_(ツ)_/¯

Singleton to standardowa praktyka jeżeli masz jakiś stateless service, po jaką cholerę dependency container miałby ci wypluwać za każdym razem nową instancję? Nie jest to może klasyczna implementacja z książki GoF, ale funkcjonalnie na jedno wychodzi. Nie wiem

[boryspo]

Nie jest to może klasyczna implementacja z książki GoF, ale funkcjonalnie na jedno wychodzi.

@croppz: to jest akurat oczywiste, problem jest w zależnościach pojawiających się nagle gdzieś w środku kodu i ogólnej upierdliwości w testowaniu tego. W przypadku klasycznego DI widzisz dokładnie co masz na wejściu i możesz to łatwo podmienić. Pół biedy jak znasz dobrze kod i wiesz czego się spodziewać, ale singletony w czymś nowym to jest koszmar.

[croppz]

problem jest w zależnościach pojawiających się nagle gdzieś w środku kodu i ogólnej upierdliwości w testowaniu tego. W przypadku klasycznego DI widzisz dokładnie co masz na wejściu i możesz to łatwo podmienić.

@boryspo: mógłbyś trochę doprecyzować o co chodziło? Bo nie jestem pewien czy rozumiem.

"Klasyczne DI" rozumiem właśnie jako dependency container wstrzykujący w konstruktor odpowiednią klasę na podstawie typu parametru (czy tam interfejsu, jeżeli w konfiguracji zmapujemy ten interfejs na jakąś

[goodghost]

@Reevo: firma zewnętrzna wzięła kasę za review, więc do czegoś p-------c się musieli, bo gdyby napisali, że wszystko jest super i nie zgłosili żadnych uwag to w oczach zleceniodawcy mogliby wyglądać nieprofesjonalnie, a Ty od pół roku nie śpisz z tego powodu. Stary, zluzuj majty i rób swoje, a review jakiegoś randoma się nie przejmuj.

[avr250]

@Reevo: Twój wpis brzmi jak bait/pasta. Jeśli jest na serio to po prostu wyluzuj. W review nie zawsze komentarze są sensowne. Recenzent tez może się mylić. Nie analizował bym reviews sprzed pół roku jak pisma świętego, albo innej prawdy objawionej. W Twojej ocenie tylko 2 komentarze były sensowne i prawdopodobnie tak właśnie było.