[nieocenzurowany88]

Mirki czy taka klasa do walidacji numeru pesel jest ok? Nie chodzi o logikę, bo działa tylko o to czy nie jest za dużo #!$%@? w jednej klasie i czy nie powinno się czegoś takiego porozdzielać na metody. Jak coś to nie jestem programistą, tylko jestem studentem i piszę coś w springu w ramach nauki i tylko ja widzę swój kod, w sumie nikt go nie ogląda, bo na uczelni prowadzący ma #!$%@? i sprawdza tylko czy działa xD
Stąd pytanie, będę wdzięczny za wszelkie uwagi dotyczące stylu pisania.

https://pastebin.pl/view/2f0a9dfd

#naukaprogramowania #java

[Zarix]

@nieocenzurowany88: Cześć, po pierwsze moim zdaniem wyjście z funkcji powinno być w jednym miejscu, a nie w środku kodu tak będzie ładniej. Podziel każde sprawdzenie na osobną funkcje (np isValidDay), zrób zmienną przechowującą wynik (domyślnie true), która będzie zwracana na końcu i zmieniana jeśli, któraś z metod walidujących zwróci Ci false. Tak bym to widział.

Edit:
W pseudokodzie:

result = true
if(!isValidDay)
result = false
else if (!isValidCośTam)
result = false

[kinemator]

Zobacz np. to: masz zmienną length a tak naprawdę to jest długość poprawnego peselu więc czytelniejsza nazwa była by validLength, correctLength.

Masz tam kilka fragmentów jak liczenie sumy kontrolnej i różnego rodzaju walidacje. Warto to podzielić na metody których nazwa dokładnie mówi co sprawdzamy. Wtedy ktoś analizując główną metodę widzi tylko proste nazwy metod typu HasCorrectCheckSum, HasCorrectLength, HasCorrectDate a nie od razu szczegóły implementacji.

Poszczególne metody będą łatwiejsze do analizowania i krótsze

[kinemator]

@Zarix: Negacje warunków są dla czytającego trochę trudniejsze do ogarnięcia więc lepiej wg mnie by było

if (isValidDay && isValidCośTam &&....)

[Zarix]

@kinemator: No to już moim zdaniem kwestie indywidualne, bo ten if się mega rozrośnie i znowu będzie problem z brakiem czytelności, a kod gdzie krok po kroku sprawdzany jest warunek wydaje się bardziej intuicyjny.

[kinemator]

@Zarix: Jeżeli masz dobrze nazwane metody to taki if czyta się prawie jak zdanie w języku naturalnym.
Zresztą zamiast if może już nawet być return (IsValidDay && ... &&).
Tutaj był by 4 metody chyba: IsValidLength, IsAllDigits, IsValidDate, IsCorrectCheckSum więc do przeżycia.

Samo unikanie negowania warunków jest sugerowane w różnych regułach czystego kodu.
https://moderatemisbehaviour.github.io/clean-code-smells-and-heuristics/general/g29-avoid-negative-conditionals.html

[s.....e]

@Zarix: nie ma sensu tak robić, skoro 'false' w jednym przypadku powoduje, że całość powinna zwrócić false. Od razu return false zamiast sprawdzać kolejne warunki które nic nie wniosą. Jeśli dojdzie do końca to return true.

[kinemator]

@szwe: Kolejne warunki nie będą sprawdzane bo Zarix użył else if

[Zarix]

@kinemator: o proszę, szczerze nawet nie wiedziałem/zapmniałem. W tym wypadku faktycznie nie będzie takiej tragedii z ilością warunków.

@szwe: Tak jak napisał @kinemator, jeśli jeden z warunków zostanie spełnionych nie sprawdzamy dalej, pesel jest niepoprawny.

[s.....e]

@Zarix: a dobra, faktycznie. Jakoś dziwnie popatrzyłem.

[krasnoludkolo]

@nieocenzurowany88: pesel to śmieszny temat, bo część ludzi ma niepoprawny ( ͡° ͜ʖ ͡°)

[draxgar]

moim zdaniem wyjście z funkcji powinno być w jednym miejscu, a nie w środku kodu tak będzie ładniej

@Zarix: kwestia gustu, ale doświadczenie na różnym kodzie, pokazuje że czytelniej jest jak wychodzisz z metody jak tylko nie ma sensu jej przetwarzać. Wszystko ładnie się wyrównuje do lewej, nie ma głębokich wcięć w kodzie. Łatwiej ogarnąć a przede wszystkim nic ci (z powodu błędu) nie zmieni wyniku. Mnie też uczono, żeby było

[kinemator]

@draxgar: Piszę w C# i tam konwencja jest, że metody z dużej dlatego tak napisałem.