Moj pierwszy projekt w C# - TicTacToe.

Question

Witam,

prosze o ocene mojego pierwszego projektu w C#. Czy w kodzie wystepuja rzeczy, ktorych nie robi profesjonalny programista (jakies "zle nawyki")? Co byscie zmienili? :)

Pierwsza klasa (zrobilem oddzielna klase, zeby wygladalo  to na programowanie obiektowe :) )

using System;

namespace TicTacToe_ver.beta
{
    class Game
    {
        char[,] box = new char[3, 3];
        private char Winner { get; set; }
        public void GameArrayFor()
        {
            for (int i = 0; i < 3; i++)
            {
                for (int j = 0; j < 3; j++)
                    box[i, j] = '-';
            } 
        }


        public void GameBoard()
        {
            Console.WriteLine("Tic Tac Toe ver. BETA\n");
            Console.WriteLine("Player 1 - X");
            Console.WriteLine("Player 2 - O\n");

            Console.WriteLine("   1   2   3\n");
            Console.WriteLine("1  {0} | {1} | {2}", box[0, 0], box[0, 1], box[0, 2]);
            Console.WriteLine("2  {0} | {1} | {2}", box[1, 0], box[1, 1], box[1, 2]);
            Console.WriteLine("3  {0} | {1} | {2}", box[2, 0], box[2, 1], box[2, 2]);
        }


        public void TakeMove(char player)
        {

            Console.WriteLine("\nPlayer " + player + " is typing: \n");

            int i = 0;
            int j = 0;
            string myParseString = string.Empty;

            while (!int.TryParse(myParseString, out i))
            {
                Console.Write("Choose row: ");
                myParseString = Console.ReadLine();
            }

            myParseString = string.Empty;

            while (!int.TryParse(myParseString, out j))
            {
                Console.Write("Choose column: ");
                myParseString = Console.ReadLine();
            }

            if (IsMoveAvailable(i, j))
            {
                box[--i, --j] = player;
            }
            else
            {
                Console.Clear();
                GameBoard();
                Console.WriteLine("\nThis move is not available. Choose other one!");
                TakeMove(player);
            }
        }


        public bool IsMoveAvailable(int i, int j)
        {
            if (i > 3 || j > 3)
            {
                return false;
            }

            if (box[--i, --j] == '-')
            {
                return true;
            }
            return false;
        }


        public bool AllFieldsAreSet()
        {
            if (box[0, 0] != '-' && box[0, 1] != '-' && box[0, 2] != '-'
                && box[1, 0] != '-' && box[1, 1] != '-' && box[1, 2] != '-'
                && box[2, 0] != '-' && box[2, 1] != '-' && box[2, 2] != '-')
            {
                return true;
            }

            return false;
        }


        public bool WinnerRows()
        {
            //check rows
            for (int i = 0; i < 3; i++)
            {
                if ((box[i, 0] == box[i, 1]) && (box[i, 1] == box[i, 2]))
                {
                    Winner = box[i, 0];
                    if (Winner != '-')
                    {
                        return true;
                    }
                }
            }
            return false;
        }


        public bool WinnerColumns()
        {
            //check columns
            for (int j = 0; j < 3; j++)
            {
                if ((box[0, j] == box[1, j]) && (box[1, j] == box[2, j]))
                {
                    Winner = box[0, j];
                    if (Winner != '-')
                    {
                        return true;
                    }
                }
            }
            return false;
        }


        public bool WinnerDiagonal()
        {
            //check diagonal

            if ((box[0, 0] == box[1, 1]) && (box[1, 1] == box[2, 2]) ||
                (box[2, 0] == box[1, 1]) && (box[1, 1] == box[0, 2]))
            {
                Winner = box[1, 1];
                if (Winner != '-')
                {
                    return true;
                }
            }
            return false;
        }


        public bool IsWinner()
        {
            if (WinnerRows() == true || WinnerColumns() == true || WinnerDiagonal() == true)
            {
                Console.WriteLine("\nPlayer " + Winner + " won the game\n");
                return true;
            }
            return false;
        }


        public bool IsDraw()
        {
            if (AllFieldsAreSet() == true && IsWinner() == false)
            {
                Console.WriteLine("\nNobody wins! DRAW!\n");
                return true;
            }
            return false;
        }


        public bool PlayAgain()
        {
            Console.WriteLine("Do you want to play again?  'Y' - yes : 'N' - no");
            string sign = Console.ReadLine();

            if (sign.ToUpper() == "Y")
            {
                return true;
            }
            else if (sign.ToUpper() == "N")
            {
                Console.WriteLine("\nThank you for playing!");
                return false;
            }
            else
            {
                Console.Clear();
                Console.WriteLine("Something gone wrong!");
                return PlayAgain();
            }
        }
    }
}

 

Druga klasa - PROGRAM

using System;

namespace TicTacToe_ver.beta
{
    class Program
    {
        static void Main(string[] args)
        {
            Console.Clear();
            Game showBoard = new Game();
            
            showBoard.GameArrayFor();
            showBoard.GameBoard();

            char currentPlayer = 'X';

            do
            {
                showBoard.TakeMove(currentPlayer);
                Console.Clear();
                showBoard.GameBoard();

                currentPlayer = currentPlayer == 'X' ? 'O' : 'X';
            }
            while (!showBoard.IsDraw() && !showBoard.IsWinner());
            
            if (showBoard.PlayAgain())
            {
                Main(args);
            }

            Console.ReadLine();
        }
    }
}

 

 

Answer 1

No rzeczywiście to tylko wygląda na programowanie obiektowe dzięki klasom. Jednak to nie czyni tego programowaniem obiektowym. 

 public bool IsMoveAvailable(int i, int j)
        {
            if (i > 3 || j > 3)
            {
                return false;
            }
 
            if (box[--i, --j] == '-')
            {
                return true;
            }
            return false;
        }

Można spokojnie zamienić na:

 public bool IsMoveAvailable(int i, int j)
        {
            if (box[--i, --j] != '-')
            {
                return false;
            }
            return true;
        }

A to możesz załatwić spokojnie pętlą:

public bool AllFieldsAreSet()
        {
            if (box[0, 0] != '-' && box[0, 1] != '-' && box[0, 2] != '-'
                && box[1, 0] != '-' && box[1, 1] != '-' && box[1, 2] != '-'
                && box[2, 0] != '-' && box[2, 1] != '-' && box[2, 2] != '-')
            {
                return true;
            }
 
            return false;
        }

Jakąś tam masz pewnie w C#.

Nie jest tragicznie, ale jeszcze długa droga. Ja bym inaczej rozplanował klasy. Zrobiłbym klasę Board, interfejs Player który byłby implementowany przez Playera #1 i #2. Zrobiłbym klasę walidatora. Do tego można pomyśleć o rozszerzeniu tego na dowolną liczbę pól. Dodatkowo, w celu ciekawostki, możesz spróbować wykorzystać "magiczny kwadrat" do weryfikacji wygranej. Ze względów optymalizacyjnych jest to sprawniejsze rozwiązanie. Nieco inaczej wygląda to w przypadku planszy o dowolnej liczbie pól. Trzeba byłoby przeanalizować, czy rozwiązanie z milionem ifów, czy może generowanie w locie liczby do magicznego kwadratu jest szybsze.