Zipper z single thread executor - ocena kodu

Zipper z single thread executor - ocena kodu
Burdzi0
  • Rejestracja:prawie 9 lat
  • Ostatnio:7 dni
  • Lokalizacja:Futurama
  • Postów:887
0

Witajcie!

Mój pierwszy jakikolwiek review przez kogokolwiek - nie zostawcie na mnie suchej nitki (i tak ostatnio moja wiara w siebie zniknęła gdzieś w niewyjaśnionych okolicznościach, więc bez różnicy).
Chciałbym tylko zauważyć, że moim celem było stworzenie zippera, który dodawałby pliki jeden po drugim, każdy na nowym wątku (jako, że ćwiczę Concurrency).

Source: https://bitbucket.org/Burdzi0/commandzip/src


Bite my shiny metal ass!
Life throws you an error code like that, you don't have the luxury of a ZnVja2luZw== pop-up explanation *Robię projekty studenckie, pisz priv ;) *
0

Za mało kodu, żeby konkretnie ocenić.

Burdzi0
Polecam w takim razie pozostałe repozytoria
panryz
  • Rejestracja:około 17 lat
  • Ostatnio:około 8 godzin
0

Nic nie jest za mało.

  1. Ta metoda main to nie może mieć wydzielonych mniejszych metod które opiszą co się gdzie dzieje zamiast tych komentarzy?
  2. Magic numbers
  3. W tym forze w main nie da rady jechać od 0? Ja wiem że potem porównujesz argument większy o 1, ale jakoś brzydko to wygląda
  4. Co się stanie jeśli w klasie ZipFile, File będzie nullem?
  5. Metoda zipIt patrz punkt 1.
  6. Ten try catch wygląda tragicznie.
  7. Tak samo klasa ZipStreamManager znajduje zastosowanie w punktach wyżej.
  8. Czy jesteś w stanie do tego kodu dopisać unit testy?
Burdzi0
  • Rejestracja:prawie 9 lat
  • Ostatnio:7 dni
  • Lokalizacja:Futurama
  • Postów:887
0

@panryz Średnio potrafię dzielić metody na mniejsze. Co polecasz z try ... catch? Nie umiem pisać unit testów, w ogóle testów.


Bite my shiny metal ass!
Life throws you an error code like that, you don't have the luxury of a ZnVja2luZw== pop-up explanation *Robię projekty studenckie, pisz priv ;) *
panryz
Średnio potrafię dzielić metody na mniejsze średnio to ja to zdanie ogaraniam :/ Co do try...catch to java 7 miała taki ficzer: http://docs.oracle.com/javase/7/docs/technotes/guides/language/catch-multiple.html nie wspominając już o tym że mamy jave 8 https://docs.oracle.com/javase/tutorial/essential/exceptions/tryResourceClose.html A no i koniecznie pomyśl o tym unitach, a może zaczniesz korzystać z interfejsów. Niby mały program, ale można to otestować.
Burdzi0
@panryz Nie umiem dzielić problemu na mniejsze, rozkładać na czynniki pierwsze, tworzyć metody wystarczająco małe, aby faktycznie wykonywałyby jedno zadanie, jednocześnie zachowując pełną spójność, naturalność korzystania z nich.
Burdzi0
I do którego catch się odnosisz (czy do wszystkich)?
panryz
Stworzyłeś metodę zipIt, openStream coś tam jeszcze, to potrafisz zrobić inną która opisze mniejszy kawałek kodu. Wszystkie try..catche

Zarejestruj się i dołącz do największej społeczności programistów w Polsce.

Otrzymaj wsparcie, dziel się wiedzą i rozwijaj swoje umiejętności z najlepszymi.