Cześć
Proszę o ocenę mojego programu w konsoli:
Funkcjonalności :
W trakcie:
artur52 napisał(a):
- Odpalam apkę, dopisuje adres "Moj adres" a w adresach jest "moj" -.-
Błąd zabawny nie powiem, ale to dowodzi na to że muszę być bardziej rozważny w testowaniu aplikacji. Błąd został naprawiony :)
artur52 napisał(a):
- Restart apki z tymi danymi co wpisałem wcześniej i kompletny błąd wczytania (4 razy imię zamiast danych)
Mógłbyś powiedzieć mi co wpisałeś, czy już nie pamietasz?
artur52 napisał(a):
- Zamiast pisać CSVUtils to było użyć libki do tego chyba, że w ramach ćwiczeń.
Ćwiczeń.
artur52 napisał(a):
- Jakaś masakra w modyfikatorach dostępu
Chodzi ci że raz jest private, a raz nieuzasadnione protected?
artur52 napisał(a):
- List<Person> sth = new ArrayList<>(); Nie musisz podawać typu po raz drugi. W wielu miejscach kodzie tak masz.
Rozumiem, dzięki za uwagę robiłem tak wcześniej bo myślałem że jest to bardziej czytelne lecz teraz uznaje że to faktycznie bez sensu.
artur52 napisał(a):
- wiele nieużywanych zmiennych
Znalazłem tylko jedną. Jeśłi chodzi o enumy, dałem wszystkie klawiszę, nawet jak jakiegoś nie używam. Nie chce dodawać co nowy ekran, nowego klawisza. poza tym takiego enuma moge bez problemowo użyć w innym projekcie.
artur52 napisał(a):
- validator niewiele co sprawdza
Cóż, trudno się mówi
artur52 napisał(a):
- do Person zrób Builder albo konstruktor z wszystkimi arg. Setowanie po kolei zmniejsza czytelność i powoduje rozwleczenie kodu. Ponadto mając konstruktor, nie zapomnisz po dodaniu jakiegoś pola zmienić implementacji i nie poleci niespodziewany null.
Co do konstruktora nie chciałbym żeby miał za dużo parametrów. Co do buildera słuszna uwaga.
artur52 napisał(a):
- jakieś testy ?
Nie ma testów, lecz mam w planie je wprowadzić.
artur52 napisał(a):
- używaj mvn do projektu. Nawet tak małego. Wtedy jest spójna struktura + łatwo dodawać jakieś zewnętrzne libki.
j/w
M4v3n napisał(a):
artur52 napisał(a):
- Restart apki z tymi danymi co wpisałem wcześniej i kompletny błąd wczytania (4 razy imię zamiast danych)
Mógłbyś powiedzieć mi co wpisałeś, czy już nie pamietasz?
Artur Nazwisko 123123 Moj adres
Moj adres dosłownie 'moj adres'
artur52 napisał(a):
- Jakaś masakra w modyfikatorach dostępu
Chodzi ci że raz jest private, a raz nieuzasadnione protected?
Tak, IntelliJ generalnie w wielu miejscach pokazuje warningi odnosnie tego kodu. Aż dziwne, że wrzuciłeś go w takim stanie
artur52 napisał(a):
- wiele nieużywanych zmiennych
Znalazłem tylko jedną. Jeśłi chodzi o enumy, dałem wszystkie klawiszę, nawet jak jakiegoś nie używam. Nie chce dodawać co nowy ekran, nowego klawisza. poza tym takiego enuma moge bez problemowo użyć w innym projekcie.
Czasem przekazujesz zmienną do metody a potem od razu ja przypisujesz do innej zmiennej albo niepotrzebnie robisz zmienną pośrednią w przekazywaniu do jakiegoś infa czy czegoś takiego
artur52 napisał(a):
- do Person zrób Builder albo konstruktor z wszystkimi arg. Setowanie po kolei zmniejsza czytelność i powoduje rozwleczenie kodu. Ponadto mając konstruktor, nie zapomnisz po dodaniu jakiegoś pola zmienić implementacji i nie poleci niespodziewany null.
Co do konstruktora nie chciałbym żeby miał za dużo parametrów. Co do buildera słuszna uwaga.
Ile dla Ciebie jest zbyt dużo?
Dzięki za recenzje. Nie mam pojęcia jak mógł się wkraść tam taki głupi błąd ;/.
Mam jeszcze jedną prośbę. Do tworzenia obiektów zaimplementowałem wzorzec fluent builder. Myślisz że to jest ok?
Jak najbardziej :) tylko implementacja taka trochę niezbyt Ci wyszła moim zdaniem.
Powinieneś to zaimplementować tak aby wywoływać to w stylu:
Person p = Person.builder().param1().param2().build();
A u Ciebie wyszło takie trochę nie wiadomo co :(