Skip to main content
fix typos and grammar
Source Link
ChrisWue
  • 20.4k
  • 4
  • 42
  • 107

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 youthe methods have unclear names and/or unclear parameter names for. 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.

  1. 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 your 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 gamesgame) 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 it'sits changes. INIn 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).

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 you 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.

  1. 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 your 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 games) 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 it's 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).

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.

  1. 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).

added some more explanation
Source Link
ChrisWue
  • 20.4k
  • 4
  • 42
  • 107

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 you 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.

  1. 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 your 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 games) 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 it's 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).

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 you 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.

  1. 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 your 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 games) 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 it's changes. 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).

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 you 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.

  1. 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 your 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 games) 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 it's 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).

Source Link
ChrisWue
  • 20.4k
  • 4
  • 42
  • 107

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 you 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.

  1. 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 your 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 games) 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 it's changes. 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).