Ocena kodu bardzo prostej aplikacji

0

Cześć,

Od 2-3 tygodni uczę się Javy, po złapaniu podstaw zastosowałem się do tego, co większość osób pisze na temat efektywnej nauki programowania, czyli spróbowałem sam napisać aplikację, ucząc się na popełnionych błędach i szukając wskazówek na stackoverflow etc.. Jako że jestem amatorskim biegaczem, postanowiłem, że najprzyjemniej będzie jak napiszę coś, co może mi się przydać, czyli "kalkulator tempa biegu". Aplikacja jest konsolowa, wybiera się jeden z czterech dystansów (5 km, 10 km, półmaraton, maraton), potem zgodnie z poleceniami wprowadza czas, jaki chce się osiągnąć, a program podaje, w jakim tempie trzeba biec każdy kilometr, aby osiągnąć podany czas.

https://github.com/maciejsyp/RunningPaceCalculator

Aplikacja jest bardzo prosta i jeśli ktoś będzie miał chwilę, by rzucić okiem i podpowiedzieć, co można poprawić/ulepszyć, co jest źle zrobione etc., to będę bardzo wdzięczny. Bo że nawet tak mały program da się zrobić inaczej i lepiej to nie mam wątpliwości :)

W niedalekiej przyszłości mam zamiar przepisać program tak, żeby nie był konsolowy, a okienkowy (uczę się podstaw Java FX, zrobiłem już nawet jeden mały program okienkowy) oraz wprowadzić odwrotną możliwość wyliczeń, czyli podajesz tempo na km, wybierasz dystans, a program wylicza czas.

Dziękuję z góry i pozdrawiam!

0

Jeżeli ktoś już zaglądał, to dokonałem pewnych zmian - zmieniłem switcha na ify i program już reaguje prawidłowo, kiedy ktoś wybierze liczbę spoza zakresu dostępnego w menu (jak tak sobie teraz myślę, to mogłem zrobić to samo, zostając przy switchu). Teraz muszę się jeszcze zająć tym, że ktoś może wprowadzić literę lub inny znak i żeby to nie wywalało programu.

0
  1. widac ze nie masz pliku gitignore albo go zle uzywasz. Nie komituje sie binarnych (wiec .class)
  2. tak samo nie komituje sie ustawien IDE
  3. Main za duzy
  4. magic numbers
  5. DRY. Masz ten sam komunikat tylko z jednym innym parametrem
  6. Zapoznaj sie z FactoryPattern
  7. nie obslugujesz bledow. Jezeli wprowadze bla to CI sie wszystko wykrzaczy
  8. Brak testow
  9. praktycznie wszystkie obliczenia w maratonach sa takie same. Znowu DRY
0
fasadin napisał(a):
  1. widac ze nie masz pliku gitignore albo go zle uzywasz. Nie komituje sie binarnych (wiec .class)
  2. tak samo nie komituje sie ustawien IDE
  3. Main za duzy
  4. magic numbers
  5. DRY. Masz ten sam komunikat tylko z jednym innym parametrem
  6. Zapoznaj sie z FactoryPattern
  7. nie obslugujesz bledow. Jezeli wprowadze bla to CI sie wszystko wykrzaczy
  8. Brak testow
  9. praktycznie wszystkie obliczenia w maratonach sa takie same. Znowu DRY

Dziękuję, że miałeś czas spojrzeć. Tak jak wspominałem na początku, jestem jeszcze bardzo zielony w temacie.

1,2) poczytam o tym, obsługi githuba też się dopiero uczę
3) rozumiem, że wszystko, co się da powinienem wynieść do innych klas?
4) nie rozumiem, ale zaraz wyguglam :)
5,9) popracuję nad tym, żeby nie powtarzać kodu
6) ok, dziękuję
7) obsłużyłem InputMismatchException
8) do tego, czym są testy i jak się je robi jeszcze na moim etapie nauki nie dotarłem

0

Oprócz uwag powyżej dodałbym:

  • to dobry przykład na wykorzystanie enuma np.:
enum Distances {
        KILOMETERS_5(5000),
        KILOMETERS_10(10000)
        ...;
        
        private int distance;
        
        Distances(int distance){
            this.distance = distance;
        }
        
        public int countPace(long timeInSeconds){
            return (int) timeInSeconds/this.distance;
        }
    }
  • unifikuj jednostki wewnątrz swojej aplikacji, po co komplikujesz obliczenia, przekazując do countPace czas rozbity na sekundy, minuty, godziny, pobierz od użytkownika czas w dowolnej formie, przelicz np. na sekundy, obliczenia wykonuj na sekundach, a dopiero przy prezentacji konwertuj na formę dogodną dla użytkownika np. godzinysekundy.
0

Cześć ponownie,

Wprowadziłem kilka zmian do aplikacji, korzystając z Waszych uwag. Jeżeli będziecie mieli chwilę, żeby spojrzeć, czy jest lepiej i co jeszcze poprawić, będę wdzięczny :)

Pozdrawiam!

0

A po co Ci klasy Marathon, HalfMarathon itd, łącznie z klasą abstrakcyjną RunningDistances, różnią się tylko jednym polem właśnie z enumem. Po to zaproponowałem enuma, że on zastępuje obecność tych wszystkich klas i ich obiektów, a dokładniej Distances to stałe, które są obiektami odpowiedzialnymi za każdy konkretny dystans. Całe obliczenia, które robi program wykonuje metoda coutPace, której przekazujesz czas biegu a ona zwraca tempo adekwatne do dystansu, tak jak zrobiłeś. Według mnie cała ta fabryka nic nie wnosi, dodanie nowego dystansu, wymaga nie tylko zrobienia dodatkowej stałej w enum ale zdefiniowanie następnej klasy itd.
W metodzie main wystarczy w każdym if-ie rozpoznającym jaki dystans wybrał użytkownik dodać:
System.out.println(Distances.HALFMARATHON.countPace(ConvertToSecond(time)));
Do operowania i wprowadzania czasu możesz wykorzystać klasę Duration, a wtedy ConvertToSecond nie będzie Ci potrzebne. Do wyświetlania menu z wariantami dystansów, możesz w enum Distances dodać pole String label i metodę getLabel(), a wtedy

for(Distances dist: Distances.values()) {
	System.out.println(dist.getLabel());
}

Do wprowadzania czasu wystarczy wczytać łańcuch w formacie akceptowanym prze Duration. Jak dobrze pokombinujesz z nazwami stałych w enum Distances to też ten potworek z if-ami w main nie będzie potrzebny.

0

Przekombinowałem, bo chciałem wprowadzić równocześnie poprawki wskazane przez Was obu. Próbowałem wprowadzić simple factory pattern (stąd "superklasa" RunningDistances i subklasy przez nią tworzone dla poszczególnych dystansów w DistancesFactory), a faktycznie wystarczy do tego enum. Pokombinuję jeszcze z tym, o czym teraz pisałeś. Dzięki!

1 użytkowników online, w tym zalogowanych: 0, gości: 1