Program Cleaner .NET

no_solution_found napisał(a):

no to ja od siebie mogę dodać takie uwagi:

• masz za dużo bloków jeden w drugim. Przykład: https://github.com/lukaszpl/Cleaner-.NET/blob/master/Cleaner%20.NET/Cleaner%20.NET/Cleaning.cs#L36 Jak masz już 2 bloki zagnieżdżone, to POWAŻNIE się zastanawiaj nad trzecim. Jak masz 5, to jest pewne, że robisz to źle
• jest dużo powtarzającego się kodu, np Cleaning::ClearTemporaryFiles oraz ::ClearWindowsErrorReporting są do siebie bardzo podobne
• funkcje robią za dużo i zapewne stąd ta ilość zagnieżdżeń, ja bym jeszcze bardziej to rozbił, a operacje usuwania plików w folderze wyrzuciłbym do oddzielnej klasy
• w ogóle klasa Cleaning jest za duża! 200-300 linii z komentarzami to powinien być max

Wybacz, że pytam dopiero teraz ale wczoraj nie miałem już siły nad tym myśleć ;)

1. Czemu zagnieżdżanie dużej ilości bloków jest błędem ? Tak się po prostu przyjęło, czy pogarsza to wydajność ?
2. Co do powtarzającego się kodu, cóż starałem się skurczyć kod programu i udało mi się to w jakimś stopniu względem poprzedniej wersji, ale do doskonałości dużo brakuje (zwłaszcza w tejże klasie). Trochę nad tym posiedzę i mam nadzieje, że coś poradzę ;)
3. (4) Czemu klasy mają być tak małe ? Z powodu estetyki, czy oszczędzania pamięci ?

Pozdrawiam.

Zawsze chodzi o czytelność. Programy powinny być przede wszystkim pisane dla ludzi do czytania, a drugorzędnie do uruchamiania przez komputery.

nawet nie tylko chodzi o czytelność, ale to też jest bardzo ważny powód, a powodów jest kilka

• ww. czytelność. Jak się na raz patrzy na małą ilość kodu, to jakoś łatwiej jest w niej znaleźć to czego się szuka
• metody są mniej skomplikowane za czym idzie fakt, że jak będziesz musiał Ty albo ktoś inny tę metodę zmienić, to łatwiej przyjdzie zrozumienie co ona dokładnie robi
• mniejsze klasy/metody dużo łatwiej i wydajniej się testuje (mam na myśli Unit Test)
• w skrajnych przypadkach dochodzi do jakiś kolosów w stylu https://github.com/tcpdf-clone/tcpdf/blob/master/tcpdf.php które czasem online edytor nie jest w stanie wyświetlić :)
• jak są mniejsze klasy/metody to dużo łatwiej jest zorientować się po samej ich nazwie co DOKŁADNIE ona robi. Np powiedzmy, że mamy metodę cleanUpHistory(), która czyści historię przeglądarek. Widząc sam nagłówek jesteś w stanie powiedzieć jakie przeglądarki czyści? Co się stanie jak czyszczenie jednej z nich zakończy się niepowodzeniem (wyjątkiem). W ten sposób zrobi się tam jedna wielka ifologia z masą try->catch.
• jak napiszesz na przykład jedną metodę do czyszczenia historii przeglądarki, którą podasz jako parametr, ale po np 5 miesiącach uznasz, że można to zrobić lepiej, to zmianę zanosisz tylko w 1 miejscu, nie 5 :)

Ogólnie rzecz biorąc - ja jak i wiele osób tutaj nie przepuściłoby tego kodu przez code review, bo na dłuższą metę ten kod byłby jednym wielkim uciemiężeniem. Rozdziel tę klasę na jakieś logiczne elementy (nie wiem jak to jest z innymi klasami - nie patrzyłem). Pobaw się trochę polimorfizmem, bo przecież klasa czyszcząca historię FF będzie bardzo podobna do tej, która czyści historię w Chrome czy Opera, co nie? Zastanów się jakie są ich elementy wspólne, aby niepotrzebnie nie powtarzać kodu.

Poczytaj sobie o co chodzi z KISS https://pl.wikipedia.org/wiki/KISS_(regu%C5%82a), DRY https://pl.wikipedia.org/wiki/DRY) oraz SOLID https://pl.wikipedia.org/wiki/Solid_(programowanie_obiektowe).

Na resztę klas nie patrzyłem, bo jestem leń :) a c# znam słabo, więc co do zastosowania języka musi wypowiedzieć się ktoś inny. Ale nie poddawaj się! Dobrze Ci idzie

Dzięki za rady ;)
Postaram się to jeszcze lepiej napisać, jak tylko skończę to odezwę się ;)

PS: Niezły ten kolos napisany w php :E Nieźle zamula na początku przeglądarkę :P