Java gra obiektowo.

[pomaraqcz]

Kłaniam się,

zacząłem się uczyć javy i dochodząc do obiektowości postanowiłem stworzyć mały projekt i napisać grę w kółko i krzyżyk. Program działa, ale chciałbym, żeby ktoś spojrzał na kod i podpowiedział czy ma on w ogóle ręce i nogi, co poprawić i co najważniejsze, że jest poprawnie napisany z punkty widzenia pisania obiektowego.

Swój kod wklejam poniżej. Stworzyłem klasę 'Kołko' z polem plansza, które jest  2-wymiarową tablicą, do tego dwa pola x i y do wyboru pola na planszy-do nich tworze też gettery i settery.

W klasie są metody do wyświetlania planszy, sprawdzania czy pole nie jest już wykorzystane przez któregoś z graczy, 2 metody graczJeden i graczDwa do nadpisywania planszy (dal gracza pierwszego jest to '1' dla drugiego '10'). Poza tym jest jeszcze metoda do sprawdzania czy któryś z graczy wygrał grę.

W mainie grę zamykam w pętli while która się wykonuje aż będzie false (do tego używam zmiennej 'rozgrywka przyjmującej return meteody czyWygrana ktora zwraca true gdy nikt nie wygrał i false gdy gra powinna się skończyć').

Tak jak pisałem na początku, jestem ciekaw waszej opinii na temat jakości kodu i samego pomysłu jak to napisać. Domyślam się, że jakość kody jest niska dlatego każda radą będzie na wagę złota.

 

package com.MM.KolkoIKrzyzyk;

public class Kolko {
    private int[][] plansza = {{0, 0, 0}, {0, 0, 0}, {0, 0, 0}};
    private int x;
    private int y;


    public int getX() {
        return x;
    }

    public void setX(int x) {
        this.x = x;
    }

    public int getY() {
        return y;
    }

    public void setY(int y) {
        this.y = y;
    }

    public void wyswietlPlansze() {
        for (int i = 0; i < plansza.length; i++) {
            for (int j = 0; j < plansza.length; j++) {
                System.out.format(" | %2d", (plansza[i][j]));
            }//koniec fora dla j
            System.out.print("|");
            System.out.println();

        }//koniec fora dla i
        System.out.println();
    }//koniec metody wyswietlPlansze

    public boolean sprawdz() {
        if (plansza[this.x - 1][this.y - 1] == 1 || plansza[this.x - 1][this.y - 1] == 10) {
            System.out.println("To pole jest juz zajete, postaraj sie jeszcze raz!");
            return false;
        } else return true;
    }

    public boolean ruchGraczJeden() {
        if (sprawdz()) {
            plansza[this.x - 1][this.y - 1] = 1;
            wyswietlPlansze();
            return false;
        } else {
            wyswietlPlansze();
            return true;
        }
    }

    public boolean ruchGraczDwa() {
        if (sprawdz()) {
            plansza[this.x - 1][this.y - 1] = 10;
            wyswietlPlansze();
            return true;
        } else {
            wyswietlPlansze();
            return false;
        }
    }

    public boolean czyWygrana() {

        int warunek = 0;
        //czy wygrwa jakis wiersz
        for (int i = 0; i < plansza.length; i++) {
            for (int j = 0; j < plansza.length; j++) {
                warunek = warunek + plansza[i][j];
            }
            if (warunek == 3) {
                System.out.println("wygral gracz 1");
                return false;
            } else if (warunek == 30) {
                System.out.println(" wygral gracz 2");
                return false;
            }
        }

        //czy wygrywa jakis rzad

        warunek = 0;
        for (int i = 0; i < plansza.length; i++) {
            for (int j = 0; j < plansza.length; j++) {
                warunek = warunek + plansza[j][i];
            }
            if (warunek == 3) {
                System.out.println("wygral gracz 1");
                return false;
            } else if (warunek == 30) {
                System.out.println(" wygral gracz 2");
                return false;
            }
        }
        return true;
    }


}//koniec klasy kolko



package com.MM;

import com.MM.KolkoIKrzyzyk.Kolko;

import java.util.Scanner;

public class Main {

    public static void main(String[] args) {
        // write your code here
        Kolko gra = new Kolko();

        boolean tura = true;
        boolean rozgrywka = true;


        while (rozgrywka) {
            if (tura) {
                System.out.println("======TURA GRACZA JEDEN======");
                System.out.println("Podaj wiersz: ");
                gra.setX(new Scanner(System.in).nextInt());
                System.out.println("Podaj rzad: ");
                gra.setY(new Scanner(System.in).nextInt());
                tura = gra.ruchGraczJeden();
                rozgrywka = gra.czyWygrana();


            } else {
                System.out.println("======TURA GRACZA DWA======");

                System.out.println("Podaj wiersz: ");
                gra.setX(new Scanner(System.in).nextInt());
                System.out.println("Podaj rzad: ");
                gra.setY(new Scanner(System.in).nextInt());
                tura = gra.ruchGraczDwa();
                rozgrywka = gra.czyWygrana();

            }

        }


    }//koniec maina
}//koniec klasy main

 

[zsm1g92]

1. Wiersz, rząd?

2. Kod za długi (np. ruchGraczJeden = ruchGraczDwa, czyWygrana dużo kodu, a przekątnych nie sprawdza) i średnio czytelny (np. w implementacji metody main funkcjonalność, którą powinna zapewniać klasa Kolko)

3. Warunkowe wykonywanie instrukcji:

        if (sprawdz()) {
            plansza[this.x - 1][this.y - 1] = 10;
            wyswietlPlansze();
            return true;
        } else {
            wyswietlPlansze();
            return false;
        }

wcale nie takie warunkowe (wyswietlPlansze(); )

4. GUI się rozjeżdża.

5. Gdzie jest klasa gracz?

6. Podobno zalecane jest używanie Stream API i lambd zamiast pętli.

[almagesta]

Jak na kogoś, kto się dopiero uczy, to całkiem, całkiem. Kilka uwag:

1. No nie ukrywajmy, jakbyś posiadał klasę Gracz z metodą zaznaczPole(int x, int y) i z scanera podawał tylko x i y, to by kod ładniej wyglądał.

2. Dobrą praktyką jest wyrabianie sobie fachowo nazywanej idioto-odporności, czyli co się stanie, jeżeli użytkownik poda wiersz 7, a nasze kółko i krzyżyk ma tylko 3 wiersze? Albo poda ktoś "a"?

3. Plansza, jako dwuwymiarowa tablica... jak kto lubi, ale czy nie lepiej pomyśleć o planszy jak o grafie (takim matematycznym/informatycznym)? Możesz wtedy uzależnić wielkość grafu od konstruktora klasy i bezproblemowo tworzyć abstrakcyjne wersje kółka i krzyżyka, np. 4x4 albo 7x7. Chodzi tutaj przede wszystkim o to, aby było jak najmniej "hardcodu" (czyli wpisanej na sztywno wartości) - idealnie 0 hardcodu.

4. Wartości do zmiennych, np. zamiast 

if (warunek == 3)

lepiej wyglądałoby

if (warunek == PLAYER_1_WIN_VALUE)

albo najlepiej

public boolean didPlayerWon(Player player) {
    //funkcja, która dla "player" sprawdza, czy wygrał
    ...
}

i wtedy punkt 3 i natura grafowa pozwala nam sprawdzić kolorowanie węzłów grafu. A co do kolorowania...

5. Zamiast wartości 1 czy 10, to albo stosować enum (jeżeli wiemy, że zawsze będzie dwóch graczy), albo lepiej zezwolić na abstrakcję 3 lub więcej graczową i zezwolić na 

public class Player {
 private int id;
  private Random random;
  
  public Player() {
    this.id = random.nextInt();
  }
  
  public String getMarker() {
    //wygenerowanie stringa albo na bazie id albo czegoś lepszego - ważne, aby dla każdego był unikalny - tlyko tutaj już bez randoma
  }
  
}

 

Tak jak zobaczysz w innych pewnie odpowiedziach - ile ludzi, tyle pomysłów i zastrzeżeń na to, więc nie masz się też co przejmować. Ode mnie to tylko sugestie jak można to zmienić i na co zwracać uwagę. 

 

Powodzenia przy nauce javy.