Noughts and Crosses WPF app

Question

I've been learning C# and was looking for some feedback on my latest project, which is a standard Noughts and Crosses game. Two people are required to play (no AI yet).

MainWindow.xaml.cs

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
using System.Windows;
using System.Windows.Controls;
using System.Windows.Data;
using System.Windows.Documents;
using System.Windows.Input;
using System.Windows.Media;
using System.Windows.Media.Imaging;
using System.Windows.Navigation;
using System.Windows.Shapes;

namespace NaughtsAndCrossesGUI
{
    /// <summary>
    /// Interaction logic for MainWindow.xaml
    /// </summary>
    public partial class MainWindow : Window
    {
        public bool isNaughtTurn;
        public char[,] board = new char[3,3];
        public string instructions = "2 People are needed\n\nChoose your symbol:\nNaught or Cross \n\nClick to place your symbol when it's your turn.When a line of one symbol is found that symbol wins";

    public MainWindow()
    {
        InitializeComponent();
        Title = "Nauts and Crosses";
    }


    //Button Input
    #region
    //Top Buttons
    void TopLeftClick(object sender, RoutedEventArgs e)
    {
        Button(2, 0, board);
    }

    void TopMiddleClick(object sender, RoutedEventArgs e)
    {
        Button(2, 1, board);
    }

    void TopRightClick(object sender, RoutedEventArgs e)
    {
        Button(2, 2, board);
    }

    //Middle Buttons
    void MiddleLeftClick(object sender, RoutedEventArgs e)
    {
        Button(1, 0, board);
    }

    void MiddleClick(object sender, RoutedEventArgs e)
    {
        Button(1, 1, board);
    }

    void MiddleRightClick(object sender, RoutedEventArgs e)
    {
        Button(1, 2, board);
    }

    //Bottom Buttons
    void BottomLeftClick(object sender, RoutedEventArgs e)
    {
        Button(0, 0, board);
    }

    void BottomMiddleClick(object sender, RoutedEventArgs e)
    {
        Button(0, 1, board);
    }

    void BottomRightClick(object sender, RoutedEventArgs e)
    {
        Button(0, 2, board);
    }
    #endregion

    //Framework

    bool failedAttempt = false;

    void Button(int p1, int p2, char[,] board)
    {

        if (inputToGrid(p1, p2, board, isNaughtTurn))
        {
            isNaughtTurn = !isNaughtTurn;
        }

        setTurnMsg(isNaughtTurn);
        setButtons();

        if (stateCheck() && !failedAttempt)
        {
            failedAttempt = true;
            return;
        }

        if (stateCheck() && failedAttempt)
        {
            reset();
            return;
        }
    }

    void setButtons()
    {
        //Top
        TopLeft.Content = board[2, 0];
        TopMiddle.Content = board[2, 1];
        TopRight.Content = board[2, 2];
        //Middle
        MiddleLeft.Content = board[1, 0];
        Middle.Content = board[1, 1];
        MiddleRight.Content = board[1, 2];
        //Bottom
        BottomLeft.Content = board[0, 0];
        BottomMiddle.Content = board[0, 1];
        BottomRight.Content = board[0, 2];
    }

    void setTurnMsg(bool noughtTurn)
    {
        TurnDisplayer.Content = "Current turn: ";
        if (noughtTurn)
            TurnDisplayer.Content += "Nought";
        else
            TurnDisplayer.Content += "Cross";
    }

    void reset()
    {
        board = new char[3, 3];
        Instuctions.Text = instructions;
        setTurnMsg(isNaughtTurn);
        setButtons();
        failedAttempt = false;
    }

    bool stateCheck()
    {
        switch (checkBoard(board))
        {
            case 'F':
                TurnDisplayer.Content = "It's a Draw";
                Instuctions.Text = "Click any square to \nreset";
                return true;
            case 'X':
                TurnDisplayer.Content = "Cross Wins";
                Instuctions.Text = "Click any square to \nreset";
                return true;
            case 'O':
                TurnDisplayer.Content = "Nought Wins";
                Instuctions.Text = "Click any square to \nreset";
                return true;
        }
        return false;
    }

    static bool inputToGrid(int p1, int p2,char[,] grid,bool naughtTurn)
    {
        if (grid[p1, p2] == '\0')
        {
            if (naughtTurn)
                grid[p1,p2] = 'O';
            else
                grid[p1,p2] = 'X';

            return true;
        }
        return false;
    }

    static char OX(char c)
    {
        if (c.ToString().ToUpper() == "X")
        {
            return 'X';
        }
        else if (c.ToString().ToUpper() == "O")
        {
            return 'O';
        }
        return ' ';
    }

    /// <summary>
    /// Returns the state of the board in terms of Noughts and Crosses Rules
    /// O or X == Naught\Cross win
    /// F == Full board
    /// ' ' == Nothing to stop play
    /// </summary>
    static char checkBoard(char[,] board)
    {
        if ((OX(board[1, 1]) == "O"[0] || OX(board[1, 1]) == "X"[0]) && (board[2, 2] == board[1, 1] && board[1, 1] == board[0, 0]))
        {
            //Diagonal
            return OX(board[1, 1]);
        }
        else if ((OX(board[1, 1]) == "O"[0] || OX(board[1, 1]) == "X"[0]) && (board[2, 0] == board[1, 1] && board[1, 1] == board[0, 2]))
        {
            //Diagonal
            return OX(board[1, 1]);
        }
        else if ((OX(board[2, 1]) == "O"[0] || OX(board[2, 1]) == "X"[0]) && (board[2, 0] == board[2, 1] && board[2, 1] == board[2, 2]))
        {
            //Top row
            return OX(board[2, 1]);
        }
        else if ((OX(board[1, 1]) == "O"[0] || OX(board[1, 1]) == "X"[0]) && (board[1, 0] == board[1, 1] && board[1, 1] == board[1, 2]))
        {
            //Middle row
            return OX(board[1, 1]);
        }
        else if ((OX(board[0, 1]) == "O"[0] || OX(board[0, 1]) == "X"[0]) && (board[0, 0] == board[0, 1] && board[0, 1] == board[0, 2]))
        {
            //Bottom row
            return OX(board[0, 1]);
        }
        else if ((OX(board[1, 0]) == "O"[0] || OX(board[1, 0]) == "X"[0]) && (board[2, 0] == board[1, 0] && board[1, 0] == board[0, 0]))
        {
            //left collumn
            return OX(board[1, 0]);
        }
        else if ((OX(board[1, 1]) == "O"[0] || OX(board[1, 1]) == "X"[0]) && (board[2, 1] == board[1, 1] && board[1, 1] == board[0, 1]))
        {
            //middle collumn
            return OX(board[1, 1]);
        }
        else if ((OX(board[1, 2]) == "O"[0] || OX(board[1, 2]) == "X"[0]) && (board[2, 2] == board[1, 2] && board[1, 2] == board[0, 2]))
        {
            //right collumn
            return OX(board[1, 2]);
        }

        int filledTiles = 0;

        for (int i = 0; i < board.GetLength(0); i++)
        {
            for (int j = 0; j < board.GetLength(1); j++)
            {
                if (board[i, j] != '\0')
                {
                    filledTiles += 1;
                }
            }
        }

        if (filledTiles >= 9)
        {
            return 'F';
        }

        return " "[0];
    }
}
}

MainWindow.xaml

<Window x:Class="NaughtsAndCrossesGUI.MainWindow"
    xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
    xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
    Title="MainWindow" Height="604.445" Width="806.667">
<Viewbox Stretch="Uniform">
    <Grid>
        <Label x:Name="TurnDisplayer" Content="Current Turn: Cross" FontFamily="Ubuntu" FontSize="34" HorizontalAlignment="Left" Height="48" VerticalAlignment="Top" Width="353" Margin="10,96,0,0"/>

        <!--Top Row-->
        <Button x:Name="TopLeft" Content="" HorizontalAlignment="Left" Margin="368,122,0,0" VerticalAlignment="Top" Width="135" Height="143" Background="#FF32BEFF" BorderBrush="#FF32BEFF" FontSize="100" FontFamily="Ubuntu" Click="TopLeftClick"/>
        <Button x:Name="TopMiddle" Content="" HorizontalAlignment="Left" Margin="507,122,0,0" VerticalAlignment="Top" Width="135" Height="143" BorderBrush="#FF32BEFF" Background="#FF32BEFF" FontSize="100" FontFamily="Ubuntu" Click="TopMiddleClick"/>
        <Button x:Name="TopRight" Content="" HorizontalAlignment="Left" Margin="646,122,0,0" VerticalAlignment="Top" Width="136" Height="143" Background="#FF32BEFF" BorderBrush="#FF32BEFF" FontSize="100" FontFamily="Ubuntu" Click="TopRightClick"/>
        <!--Middle Row-->
        <Button x:Name="MiddleLeft" Content="" HorizontalAlignment="Left" Margin="368,269,0,0" VerticalAlignment="Top" Width="135" Height="143" BorderBrush="#FF32BEFF" Background="#FF32BEFF" FontSize="100" FontFamily="Ubuntu" Click="MiddleLeftClick"/>
        <Button x:Name="Middle" Content="" HorizontalAlignment="Left" Margin="507,269,0,0" VerticalAlignment="Top" Width="135" Height="143" Background="#FF32BEFF" BorderBrush="#FF32BEFF" FontSize="100" FontFamily="Ubuntu" Click="MiddleClick"/>
        <Button x:Name="MiddleRight" Content="" HorizontalAlignment="Left" Margin="646,269,0,0" VerticalAlignment="Top" Width="136" Height="143" BorderBrush="#FF32BEFF" Background="#FF32BEFF" FontSize="100" FontFamily="Ubuntu" Click="MiddleRightClick"/>
        <!--Bottom Row-->
        <Button x:Name="BottomLeft" Content="" HorizontalAlignment="Left" Margin="368,416,0,0" VerticalAlignment="Top" Width="135" Height="143" Background="#FF32BEFF" BorderBrush="#FF32BEFF" FontSize="100" FontFamily="Ubuntu" Click="BottomLeftClick"/>
        <Button x:Name="BottomMiddle" Content="" HorizontalAlignment="Left" Margin="507,416,0,0" VerticalAlignment="Top" Width="135" Height="143" Background="#FF32BEFF" BorderBrush="#FF32BEFF" FontSize="100" FontFamily="Ubuntu" Click="BottomMiddleClick"/>
        <Button x:Name="BottomRight" Content="" HorizontalAlignment="Left" Margin="647,416,0,0" VerticalAlignment="Top" Width="135" Height="143" Background="#FF32BEFF" BorderBrush="#FF32BEFF" FontSize="100" FontFamily="Ubuntu" Click="BottomRightClick"/>

        <TextBlock x:Name="Name" TextWrapping="Wrap" Text="NOUGHTS AND CROSSES H" HorizontalAlignment="Left" Height="81" VerticalAlignment="Top" Width="772" FontSize="65" FontFamily="Ubuntu" Margin="0,10,0,0" />
        <TextBlock x:Name="Instuctions" TextWrapping="Wrap"  Text="2 People are needed &#x0a;&#x0a;Choose your symbol: &#x0a;Naught or Cross &#x0a;&#x0a;Click to place your symbol when it's your turn When a line of one symbol is found that symbol wins" HorizontalAlignment="Left" Height="376" Margin="0,193,0,0" VerticalAlignment="Top" Width="363" FontFamily="Source Code Pro ExtraLight" FontSize="24"/>
        <Label Content="Instructions:" HorizontalAlignment="Left" Height="49" Margin="10,139,0,0" VerticalAlignment="Top" Width="334" FontFamily="Source Code Pro Black" FontSize="36" FontWeight="Bold"/>

    </Grid>
</Viewbox>

And finally, if you want to see the finished project, I have a .exe available here. I do realise that as it's an .exe you may be wary of downloading. It is the code above compiled via Visual Studio but I understand if you don't feel comfortable doing so. On a side note, is there any way to get around the "I don't want to download a .exe" problem?

Answer 1

First the simple stuff:

1. Standard naming convention for methods in C# is PascalCase while you run a mix of camelCase and PascalCase.

2. Some of the methods have unclear names and/or unclear parameter names. For example this:

   void Button(int p1, int p2, char[,] board)
   

   What is p1 or p1 and what should the method do? The names should be descriptive enough to deduce the functionality from the names.

3. These comparisons are weird: OX(board[1, 0]) == "O"[0] - OX (another unclear name) returns a char so you should compare it to one: OX(board[1, 0]) == 'O'

---

Now the bigger picture stuff:

The main issue with your solution is that UI and logic are very tightly coupled which in general is a bad thing as it makes it hard to re-use the current game logic (try making a text console, winforms or web version for that game) and also not very easy to unit test.

One very popular pattern with WPF application developers is the MVVM (Model-View-ViewModel) pattern. The basic idea is that you have a model which is completely agnostic of the UI and the UI just observes the model and its changes. In WPF this is supported by something called binding where the UI "binds" the controls to the model which alleviates the need to manually push the data from the model to the UI.

It's a big topic so I won't spill out all the details and it would be a fair amount of work to re-work the code according to it but I would encourage you to do so (could make a great series of code reviews here). There are heaps of excellent articles on code project and lots of assisting frameworks around (although you don't need a framework to follow the MVVM pattern).

Answer 2

Besides all that ChrisWue said, there's also the rather odd way the UI is structured: you use a grid but then don't define any rows or columns and instead use HorizontalAlignment, VerticalAlignment and Margin to position your elements. I'm assuming this is the result of using Design view, when it would have been better to use the XAML view.

I don't even think a Grid is the best solution here, I'd much prefer nested StackPanels, especially for the 3x3 button grid. IMHO Grid.Row="0" Grid.Column="0" is ugly and error-prone, whilst StackPanels provide an easy to follow flow.