patterncsharpMinor
Noughts and Crosses WPF app
Viewed 0 times
appwpfandnoughtscrosses
Problem
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
{
///
/// Interaction logic for MainWindow.xaml
///
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);
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
{
///
/// Interaction logic for MainWindow.xaml
///
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);
Solution
First the simple stuff:
-
Standard naming convention for methods in C# is
-
Some of the methods have unclear names and/or unclear parameter names. For example this:
What is
-
These comparisons are weird:
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).
-
Standard naming convention for methods in C# is
PascalCase while you run a mix of camelCase and PascalCase.-
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.-
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).
Code Snippets
void Button(int p1, int p2, char[,] board)Context
StackExchange Code Review Q#63598, answer score: 9
Revisions (0)
No revisions yet.