Я начинающий программист, и мне было интересно, как улучшить код консоли на C #. Это игра в крестики-нолики.
using System;
namespace TicTacToe
{
class Program
{
static string[] options = { "1", "2", "3", "4", "5", "6", "7", "8", "9" }; //stores the variables to change
static bool Playing = true; //stops the game once someone wins
static int turn = 0;
static void Main(string[] args)
{
Intro();
Board();
while (Playing) //is to stop the game once someone wins
{
if (turn%2 == 0)
{
Console.WriteLine("Player 1's turn");
}
else
{
Console.WriteLine("Player 2's turn");
}
int playerInput1;
Console.WriteLine("type in your responce player");
bool torf = int.TryParse(Console.ReadLine(), out playerInput1);
playerInput1--;
// makes sure the input is put in a valid value
if (torf && playerInput1 < 9 && playerInput1 > -1) //makes sure again the input is valid and continues
{
if (options[playerInput1] == "x" || options[playerInput1] == "o")
{
Console.WriteLine("Stop stealing othe people's place");
}
else
{
if (turn%2 == 0)
{
options[playerInput1] = "x";
turn++;
Board();
WinCondition();
Tie();
}
else
{
options[playerInput1] = "o";
turn++;
Board();
WinCondition();
Tie();
}
/*int playerInput2;
Console.WriteLine("o");
bool torf2 = int.TryParse(Console.ReadLine(), out playerInput2);
playerInput2--;
if (torf2 && playerInput2 < 9 && playerInput2 > -1)
{
if (options[playerInput2] == "x" || options[playerInput2] == "o")
{
Console.WriteLine("Stop stealing other people's space");
}
else
{
options[playerInput2] = "o";
Board();
WinCondition();
torf2 = false;
torf = false;
}
}*/
}
}
else
{
Console.WriteLine("Please input a valid expression");
}
}
}
public static void Board() // makes the board
{
Console.Clear();
Console.WriteLine(" | | ");
Console.WriteLine($" {options[0]} | {options[1]} | {options[2]}");
Console.WriteLine("_____|_____|_____ ");
Console.WriteLine(" | | ");
Console.WriteLine($" {options[3]} | {options[4]} | {options[5]}");
Console.WriteLine("_____|_____|_____ ");
Console.WriteLine(" | | ");
Console.WriteLine($" {options[6]} | {options[7]} | {options[8]}");
Console.WriteLine(" | | ");
}
public static void WinCondition()
{
if (options[0] == options[1] && options[1] == options[2])
{
Playing = false;
if(turn % 2 == 0)
{
Console.WriteLine("Congrats on winning player 1, better luck next time player 2");
}
else
{
Console.WriteLine("Congrats on winning player 2, better luck next time player 1");
}
}
else if (options[3] == options[4] && options[4] == options[5])
{
Playing = false;
if(turn % 2 == 0)
{
Console.WriteLine("Congrats on winning player 1, better luck next time player 2");
}
else
{
Console.WriteLine("Congrats on winning player 2, better luck next time player 1");
}
}
else if (options[6] == options[7] && options[7] == options[8])
{
Playing = false;
if(turn % 2 == 0)
{
Console.WriteLine("Congrats on winning player 1, better luck next time player 2");
}
else
{
Console.WriteLine("Congrats on winning player 2, better luck next time player 1");
}
}
// checks for horizontal wins
else if (options[0] == options[3] && options[3] == options[6])
{
Playing = false;
if(turn % 2 == 0)
{
Console.WriteLine("Congrats on winning player 1, better luck next time player 2");
}
else
{
Console.WriteLine("Congrats on winning player 2, better luck next time player 1");
}
}
else if (options[1] == options[4] && options[4] == options[7])
{
Playing = false;
if(turn % 2 == 0)
{
Console.WriteLine("Congrats on winning player 1, better luck next time player 2");
}
else
{
Console.WriteLine("Congrats on winning player 2, better luck next time player 1");
}
}
else if (options[2] == options[5] && options[5] == options[8])
{
Playing = false;
if(turn % 2 == 0)
{
Console.WriteLine("Congrats on winning player 1, better luck next time player 2");
}
else
{
Console.WriteLine("Congrats on winning player 2, better luck next time player 1");
}
}
// checks for vertical wins
else if (options[0] == options[4] && options[4] == options[8])
{
Playing = false;
if(turn % 2 == 0)
{
Console.WriteLine("Congrats on winning player 1, better luck next time player 2");
}
else
{
Console.WriteLine("Congrats on winning player 2, better luck next time player 1");
}
}
else if (options[2] == options[4] && options[4] == options[6])
{
Playing = false;
if (turn%2 == 0)
{
Console.WriteLine("Congrats on winning player 1, better luck next time player 2");
}
else
{
Console.WriteLine("Congrats on winning player 2, better luck next time player 1");
}
}
// checks for diagonal wins
}
public static void Intro()
{
Console.WriteLine("Welcome ton");
Console.WriteLine(@"____________ ________ ________ ");
Console.WriteLine(@"___ __/__(_)______ ___ __/_____ _______ ___ __/__________ ");
Console.WriteLine(@"__ / __ /_ ___/________ / _ __ `/ ___/________ / _ __ _ ");
Console.WriteLine(@"_ / _ / / /__ _/_____/ / / /_/ // /__ _/_____/ / / /_/ / __/");
Console.WriteLine(@"/_/ /_/ ___/ /_/ __,_/ ___/ /_/ ____/___/ ");
Console.WriteLine("n1.The game is played on a grid that's 3 squares by 3 squares.nn2.Player 1 is "X" and Player 2 is "O". Players take turns putting their marks in empty squares.nn3.The first player to get 3 of her marks in a row(horizontally, vertically or diagonally) is the winner.nn4.When all 9 squares are full, the game is over. If no player has 3 marks in a row, the game ends in a tie.nn5.You can put x or o in by typing the number you want to put it at");
Console.ReadKey(false);
Console.Clear();
}
public static void Tie()
{
if (options[0] != "1" && options[1]!= "2" && options[2] != "3" && options[3] != "4" && options[4] != "5" && options[5] != "6" && options[6] != "7" && options[7] != "8" && options[8] != "9")
{
Console.WriteLine("The game is a tie");
Playing = false;
}
}
}
}
2 ответа
Выбор хороших идентификаторов
- Когда я вижу такое имя, как
optionsЯ думаю, что игрок может выбрать что-то вроде «одиночной игры» (т. Е. Игры против компьютера) или «мультиплеера». Лучшее имя было быboardпотому что это то, что есть. torf? Означает ли это «правда или ложь»? Каждое логическое значение истинно или ложно. Это не отражает того значения, которое оно имеет в данном контексте. Лучше:isValidInt. Но я бы встроил эту переменную (см. Позже).Board(). Доска — это вещь, но здесь она обозначает действие. ЛучшеPrintBoard(). То же самое дляPrintIntro().- Поле
Playingначинается с верхнего регистра и поэтому называется PascalCase. Это зарезервировано для имен типов, имен методов и имен свойств. Видеть Стандарты кодирования C # и соглашения об именах для полного списка.
Логику и структуру можно упростить и уточнить.
Во многих местах вы рассчитываете
turn % 2 == 0. Вместо этого я предлагаю напрямую рассчитать количество игроков. Мы бы объявилиstatic int player = 0;и метод, генерирующий числа 0 и 1 по очередиprivate static void SwitchPlayer() { player = (player + 1) % 2; }или же
player = 1 - player;.Это приводит к еще одному упрощению. Первый оператор if-else можно заменить на (я использую строковая интерполяция здесь)
Console.WriteLine($"Player {player + 1}'s turn");Я бы встроил переменную, которую вы назвали
torfи объявитьplayerInputв объявление переменной вне так.if (Int32.TryParse(Console.ReadLine(), out int playerInput) && playerInput is >= 1 and <= 9) { playerInput--; // Make it a 0-based index. ... }я использовал сопоставление с образцом чтобы проверить, находится ли значение в допустимом диапазоне. Но вы можете заменить его классическим логическим выражением, если хотите, или если вы используете версию до C # 9.0.
Вы различаете два случая, делая то же самое, за небольшим исключением (показанным с новыми идентификаторами):
if (player == 0) { board[playerInput] = "x"; SwitchPlayer(); PrintBoard(); WinCondition(); Tie(); } else { board[playerInput] = "o"; SwitchPlayer(); PrintBoard(); WinCondition(); Tie(); }Это можно легко упростить, объявив новое поле
static readonly string[] playerMark = { "x", "o" };Это становится
board[playerInput] = playerMark[player]; PrintBoard(); WinCondition(); Tie(); SwitchPlayer(); // Doing this after printing winner!Даже без этого нового статического поля вы можете переместить всю строку, кроме первой, из оператора if-else:
if (player == 0) { board[playerInput] = "x"; } else { board[playerInput] = "o"; } PrintBoard(); WinCondition(); Tie(); SwitchPlayer();В
WinConditionметод проверяет условия и выводит на консоль. И это достигается путем повторения одних и тех же операторов печати снова и снова. Лучше разделить две проблемы.private static bool IsWinCondition() { return board[0] == board[1] && board[1] == board[2] || board[3] == board[4] && board[4] == board[5] || board[6] == board[7] && board[7] == board[8] || board[0] == board[3] && board[3] == board[6] || board[1] == board[4] && board[4] == board[7] || board[2] == board[5] && board[5] == board[8] || board[0] == board[4] && board[4] == board[8] || board[2] == board[4] && board[4] == board[6]; } }и назовите его (здесь еще одно упрощение)
if (IsWinCondition()) { playing = false; Console.WriteLine( $"Congrats player {player + 1}, better luck next time player {2 - player}"); }Та же проблема, что и выше, с
Tie. Кроме того, он не проверяет, есть ли у нас ничья, а только если доска заполнена. У нас есть ничья, только если в то же время у нас нет выигрышной ситуации. Поэтому я изменил метод на (требуетсяusing System.Linq;перед пространством имен)private static bool IsBoardFull() { return board.All(square => square > '9'); // Because 'x' and 'o' are greater }Чтобы эта работа была изменена, тип
boardиplayerMarkкchar[]. (Вы должны заменить соответствующие двойные кавычки на одинарные.)Видеть: Enumerable.All метод расширения.
Затем условия становятся
if (IsWinCondition()) { playing = false; Console.WriteLine( $"Congrats player {player + 1}, better luck next time player {2 - player}"); } else if (IsBoardFull()) { playing = false; Console.WriteLine("The game is a tie"); }Сейчас же
playingможно сделать локальной переменной, так как она не используется или не устанавливается в других методахbool playing = true; //stops the game once someone wins while (playing) //is to stop the game once someone wins { ... }
Полное решение (без окружающего пространства имен и класса):
static char[] board = { '1', '2', '3', '4', '5', '6', '7', '8', '9' }; //stores the variables to change
static readonly char[] playerMark = { 'x', 'o' };
static int player = 0;
static void Main()
{
PrintIntro();
PrintBoard();
bool playing = true; //stops the game once someone wins
while (playing) //is to stop the game once someone wins
{
Console.WriteLine($"Player {player + 1}'s turn");
Console.WriteLine("type in your response: ");
if (int.TryParse(Console.ReadLine(), out int playerInput) && playerInput is >= 1 and <= 9) {
playerInput--; // Make it a 0-based index.
if (board[playerInput] is 'x' or 'o') {
Console.WriteLine("Stop stealing other people's place");
} else {
board[playerInput] = playerMark[player];
PrintBoard();
if (IsWinCondition()) {
playing = false;
Console.WriteLine(
$"Congrats player {player + 1}, better luck next time player {2 - player}");
} else if (IsBoardFull()) {
playing = false;
Console.WriteLine("The game is a tie");
}
SwitchPlayer(); // Switch player after printing result.
}
} else {
Console.WriteLine("Please input a valid expression");
}
}
}
private static void SwitchPlayer()
{
player = (player + 1) % 2;
}
private static void PrintBoard() // makes the board
{
Console.Clear();
Console.WriteLine(" | | ");
Console.WriteLine($" {board[0]} | {board[1]} | {board[2]}");
Console.WriteLine("_____|_____|_____ ");
Console.WriteLine(" | | ");
Console.WriteLine($" {board[3]} | {board[4]} | {board[5]}");
Console.WriteLine("_____|_____|_____ ");
Console.WriteLine(" | | ");
Console.WriteLine($" {board[6]} | {board[7]} | {board[8]}");
Console.WriteLine(" | | ");
}
private static bool IsWinCondition()
{
return
board[0] == board[1] && board[1] == board[2] ||
board[3] == board[4] && board[4] == board[5] ||
board[6] == board[7] && board[7] == board[8] ||
board[0] == board[3] && board[3] == board[6] ||
board[1] == board[4] && board[4] == board[7] ||
board[2] == board[5] && board[5] == board[8] ||
board[0] == board[4] && board[4] == board[8] ||
board[2] == board[4] && board[4] == board[6];
}
private static void PrintIntro()
{
...
}
private static bool IsBoardFull()
{
return board.All(square => square > '9');
}
Дополнительная мысль
Вы храните игровое поле в одномерном массиве. Существуют вариации крестиков-ноликов наличие большего количества строк и столбцов. Использование двумерного массива сделало бы игру более расширяемой, потому что это позволило бы тестировать условия выигрыша и печатать доску, просто перебирая строки и столбцы. Теперь мы используем составленное вручную условие выигрыша. Это нормально, поскольку плата очень маленькая. Но делать это на досках лагера было бы утомительно и чревато ошибками.
В этом обзоре позвольте мне сосредоточиться на самой длинной части вашего кода: WinCondition.
- Попробуйте отделить проверки условий от взаимодействия с пользователем, не смешивайте их
- Попробуйте использовать более подходящие названия, например:
CheckVictory,HasAWinner,HasGameEnded, так далее. - Попробуйте разделить данные и логику во время проверки условий
Когда они не разделены
var conditions = new List<bool>
{
// checks for horizontal wins
options[0] == options[1] && options[1] == options[2],
options[3] == options[4] && options[4] == options[5],
options[6] == options[7] && options[7] == options[8],
// checks for vertical wins
options[0] == options[3] && options[3] == options[6],
options[1] == options[4] && options[4] == options[7],
options[2] == options[5] && options[5] == options[8],
// checks for diagonal wins
options[0] == options[4] && options[4] == options[8],
options[2] == options[4] && options[4] == options[6]
};
return conditions.FirstOrDefault(condition => condition));
Когда они разделены
//Data
var horizontalMarks = new List<(int, int, int)>
{
(0, 1, 2),
(3, 4, 5),
(6, 7, 8)
};
var verticalMarks = new List<(int, int, int)>
{
(0, 3, 6),
(1, 4, 7),
(2, 5, 8)
};
var diagonalMarks = new List<(int, int, int)>
{
(0, 4, 8),
(2, 4, 6)
};
var marks = horizontalMarks.Union(verticalMarks).Union(diagonalMarks);
//Logic
bool hasAWinner = false;
foreach (var (first, second, third) in marks)
{
//If all data in this three positions are the same then we have found a winner
if (new[] {options[first], options[second], options[third]}.Distinct().Count() != 1) continue;
hasAWinner = true;
break;
}
return hasAWinner;
При желании вы можете ввести вспомогательный класс вместо того, чтобы полагаться на ValueTuples.
- 1
Которые должны быть
(0, 3, 6)в первой строке послеvar verticalMarks, но я сомневаюсь, что это легче читать / поддерживать в любом случае. Если вы делаете это, потому что он более общий (и, например, может быть расширен до платы NxN), можно напрямую искать по строкам, столбцам и диагоналям, используя стандартные циклы for. Для начинающего программиста это может быть даже полезно на практике с операциями деления по модулю и целочисленного деления.— Йонатан Н.
@YonatanN Спасибо, я исправил эту строку. Пожалуйста, не стесняйтесь публиковать ответ с предложенным вами решением.
— Питер Чала

Мое решение требует
SwitchPlayerровно в одном месте и производит поочередно номера игроков 0 и 1, как и ожидалось.(player + 1) % 2(или жеplayer = 1 - player) необходимо: когда player равен 0, он производит 1, а когда player равен 1, он производит 0. Итак, я не знаю, что вы имеете в виду под «непригодным для использования» и «измененным везде». Кроме того, в логике нет дилеммы «по одному». Только для отображения добавлен один. Но вы правы, что для отображения мы могли бы использовать подход, основанный на данных; однако я бы просто использовал массив:string playerName = { "one", "two" }. Получите имя сplayerName[player].— Оливье Жако-Декомб
Мы бы напечатали номера игроков с
Console.WriteLine($"Congrats player {playerName[player]}, better luck next time player {playerName[1 - player]}");. (Мы все еще должны использовать1 - playerуловка, чтобы заполучить другого игрока.)— Оливье Жако-Декомб