Pierwszy program w Javie - potrzebna konstruktywna krytyka

0

Cześć, witam.
Napisałem program, który z założenia miał być wirtualną wersją gry w "Monopoly". Link do repozytorium: https://github.com/kamedor/monopoly1.1 . Funkcje zaimplementowane do tej pory:

  • wczytywanie danych o planszy z pliku "zapisane/danePlanszy.txt"
  • wczytywanie danych o graczach
  • zaimplementowane 9 różnych rodzajów pól na planszy oraz większości akcji dla nich (m.in.: kupowanie dla Ulic, pobieranie czynszu, wypłata 200zł za przekroczenie startu)
  • prowizoryczna wersja rozgrywki poprzez terminal.
    Proszę was o przejrzenie kodu itp. Czekam na uwagi/podpowiedzi/rady odnośnie kodu. Z góry dzięki za każdą pomoc.
    PS. Swoje "Hello world" w Javie napisałem około miesiąc temu, więc się nie załamujcie :D
1

Plików class nie wersjonuj. Dodaj out/ do .gitignore.

1
  1. Java7 to switche ze stringami których używasz ale także try-with-reosueces którego nie używasz(wygooglaj), to także NIO.2 i np: klasa Path czy Paths których także nie używasz, poczytaj o tym, warto !
  2. case "przedsiębiorstwo":
    nazwa = inFile.next();
    rodzajPola = Pole.rodzajPola.PRZEDSIEBIORSTWO;
    Plansza.addPole(rodzajPola, nazwa);
    break;
    case "start":
    rodzajPola = Pole.rodzajPola.START;
    Plansza.addPole(rodzajPola, "START");
    break;
    case "parking":
    rodzajPola = Pole.rodzajPola.PARKING;
    Plansza.addPole(rodzajPola, "PARKING");
    break;
    case "szansa":
    rodzajPola = Pole.rodzajPola.SZANSA;
    Plansza.addPole(rodzajPola, "SZANSA");
    break;
    case "kasa":
    inFile.next(); // wczytaj słowo społeczna (- niepotrzebne słowo)
    rodzajPola = Pole.rodzajPola.KASA_SPOLECZNA;
    Plansza.addPole(rodzajPola, "KASA SPOŁECZNA");
    break;
    case "więzienie":
    rodzajPola = Pole.rodzajPola.WIEZIENIE;
    Plansza.addPole(rodzajPola, "WIĘZIENIE");
    break;
    case "podatek":
    inFile.next(); // wczytaj słowo dochodowy (- niepotrzebne słowo)
    int oplata = inFile.nextInt();
    rodzajPola = Pole.rodzajPola.PODATEK_DOCHODOWY;
    Plansza.addPole(rodzajPola, "PODATEK DOCHODOWY", oplata);
    break;
    case "dworzec":
    nazwa = inFile.next();
    rodzajPola = Pole.rodzajPola.DWORZEC;
    Plansza.addPole(rodzajPola,nazwa);
    break;

    każdy taki case powinien być metodą, powtarzasz kod, źle !
    //DALEJ
    Zamiast tych casów można to rozwiązać lepiej:

    doSomething(Types.PODATEK)

Takie coś powinno się wyciągać do metody

 nazwa = inFile.next();
GraczCzlowiek graczCzlowiek = new GraczCzlowiek(nazwa);
gracze.add(graczCzlowiek);

A takie coś nie powinno się zdarzyć:

ArrayList<Gracz> gracze = new ArrayList<>();

????????
isPozwolenieNaBudowe()

To jest słabe, Effective Java 2, Item 2

 private int oplata = 0;
private Random generator = new Random();
public NieKupne(rodzajPola rodzaj, String nazwa){
this.rodzaj = rodzaj;
this.nazwa = nazwa;
this.nrPola = liczbaPol;
liczbaPol++;
}
public NieKupne(rodzajPola rodzaj, String nazwa, int oplata){
this.rodzaj = rodzaj;
this.nazwa = nazwa;
this.oplata = oplata;
this.nrPola = liczbaPol;
liczbaPol++;
}

Te metody robią zdecydowanie za dużo, jedna metoda jedna akcja.
https://github.com/kamedor/mo[...].1/blob/master/src/Gracz.java

te nazwymetod, klas, zmiennych są śmieszkowe...

<trololo> Tam faktycznie są testy, najs :D nie rób takich kitajskich taktyk.... użyj jak człowiek Junita :D </trololo>
1

Na szybko

  1. polskie znaki w nazwach metod to zuo (specjalnie bez ł, bo ł zamieniło się w kodzie na ³).
  2. InputOutputFile, za dużo kodu, za dużo odpowiedzialności, za bardzo nagmatwane. Tego kobylastego switcha można zamienić na delegację do mniejszych metod i obudować odpowiednią mapą.
  3. Kupne.getAktualnyCzynsz switch w swichu. Zewnętrzny zamień na dziedziczenie. Wewnętrzny niech będzie implementacją metody w poszczególnych podklasach.
  4. klasy TDD nie są testami. Nic w nich nie sprawdzasz. To jest taki bardziej rozbudowany dupa-debug.

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