@XardasLord, jest pewien postęp, ale zostało jeszcze dużo do zrobienia.
Przede wszystkim - dobrze, że wyodrębniłeś klasy: Game, Map, i dwa typy Playerów. Dobrze, że zrobiłeś oddzielną klasę do wyświetlania informacji na konsoli, chociaż nazwa ShowData nie jest jakaś super. Trochę się jednak gubisz, zarówno w projekcie, jak i w języku. Ale po kolei:
-
Zamiast Console.Write i \n na końcu tekstu używaj Console.WriteLine.
-
Ten warunek: while (choice != 0 || choice < 1 && choice > 2) można znacznie uprościć, przecież choice < 1 && choice > 2 nie jest chyba nigdy prawdziwe, prawda?
-
Ten Twój if z is jest dobry, nawet bardzo dobry... ale zupełnie niepotrzebny. Gra wygląda identycznie, bez względu na to kto gra. Spójrz na swoją główną pętlę gry:
// GŁÓWNA PĘTLA GRY
do
{
if (PlayPlayer(map, player1, possition))
break;
if (PlayPlayer(map, player2, possition))
break;
} while (true);
Ten sam kod powielasz dwukrotnie, po co? Po to właśnie masz wspólną klasę dla obu typów graczy, żeby klasa Game nie musiała ich wewnętrznie rozróżniać. (Na marginesie - zamiast pętli do...while możesz tu użyć po prostu while.)
-
Jestem pewien, że possition pisze się przez jedno "s". ;)
-
Spójrzmy teraz na ten fragment kodu:
if (player is HumanPlayer)
{
string playername;
playername = showdata.GetPlayer1Name();
HumanPlayer player1 = new HumanPlayer('X', playername);
playername = showdata.GetPlayer2Name();
HumanPlayer player2 = new HumanPlayer('O', playername);
Masz już utworzony obiekt gracza (player). Następnie tworzysz jeszcze dwóch graczy player1 i player2. Więc masz trzech graczy, z czego gra tylko dwóch. Nie sądzisz, że coś tu jest nie tak?
- Cały kod klasy Game można znacznie uprościć, wystarczy w nim właściwie tyle:
namespace Kolko_i_krzyzyk
{
class Game
{
private readonly ConsoleOperations consoleOperations = new ConsoleOperations();
private readonly Map map;
private readonly Player first;
private readonly Player second;
public Game(Map map, Player first, Player second)
{
this.map = map;
this.first = first;
this.second = second;
}
public void Play()
{
while(true)
{
if (PlayPlayer(first))
break;
if (PlayPlayer(second))
break;
}
}
public bool PlayPlayer(Player player)
{
int position = player.GetField(map);
map.PutSign(position, player.Sign);
consoleOperations.DrawMap(map);
if (map.CheckWin(player))
{
consoleOperations.WinMessage(player.Name);
return true;
}
if (map.CheckDraw())
{
consoleOperations.DrawMessage();
return true;
}
return false;
}
}
}
(Zmieniłem ShowData na ConsoleOperations, aby nazwa była bliższa rzeczywistości.)
Nie jest jeszcze doskonały, ale chodzi mi o ideę - kod może być dużo prostszy, bo istnienie różnych typów graczy nie zmienia zasad gry.
- Druga sprawa, po co zarówno w
HumanPlayer jak i ComputerPlayer masz taki kod:
public override char Sign
{
get
{
return this.sign;
}
}
public override string Name
{
get
{
return this.name;
}
}
Skoro to jest identyczne, to powinno być w klasie bazowej! To powinny być zwykłe właściwości, a nie abstrakcyjne, skoro ich implementacja jest identyczna.
Abstrakcyjne ma być to, co może się różnić w klasach potomnych. W tym przypadku jest to wybór pola do następnego ruchu, czyli metoda GetField i nic więcej.
-
W kilku miejscach programu sprawdzasz taki warunek:
map.ArrayMap[possition] == 'X' || map.ArrayMap[possition] == 'O'
Nie lepiej byłoby napisać metodę w klasie Map, która by sprawdzała, czy pole jest wolne? Dzięki temu niepotrzebna byłaby zupełnie właściwość ArrayMap.
-
Masz dwie takie funkcje:
public string GetPlayer1Name()
{
Console.Write("\n\nPodaj imię pierwszego gracza (X): ");
string playername = Console.ReadLine();
return playername;
}
public string GetPlayer2Name()
{
Console.Write("\n\nPodaj imię drugiego gracza (O): ");
string playername = Console.ReadLine();
return playername;
}
Różnią się między sobą jednie w dwóch miejscach. Wiesz chyba, że można to uprościć.
-
Za to pomysł wyłączania aplikacji gdy użytkownik wpisze litery zamiast cyfr bardzo mi się podoba. Od razu nauczy się tych durnych użytkowników uwagi! ;)
A tak na serio - zrób to w sposób wygodny dla użytkownika. Użyj metody int.TryParse i sprawdzaj, czy użytkownik wpisał liczbę. Jeśli nie, to poproś go o powtórkę. Ale nigdy nie wyłączaj aplikacji po błędnym zachowaniu użytkownika!
-
Klasa Map ma reprezentować planszę do gry. Czyli powinna móc:
a) powiedzieć, jaki znak jest na którym polu;
b) ustawić znak na wskazanym polu;
c) powiedzieć, czy dane pole jest wolne;
d) powiedzieć, ile wolnych pól zostało.
I to wszystko. Metody sprawdzające wynik gry powinny się znajdować w klasie Game, a nie Map. Nawet w rzeczywistym świecie, to nie kartka papieru zna zasady gry w kółko i krzyżyk.
Gdy przeniesiesz metody CheckWin i CheckDraw to klasy Game, kod się jeszcze skróci i uprości.