Pattern-finding game

This is a game that is supposed to be based on strategy and guessing.

The letters need to match in a ABCD fashion on each button on buttons[0] - buttons[3] ... and buttons[n] - buttons[n] in each square, kind of like a matrix, although there is no matrix math involved.

This is how the game starts:

Each time a user clicks the "A" button, the buttons' text changes for all the buttons[n] in the ABCD sequence, and to win you need to match each square of controls to the ABCD sequence.

Any help in how to improve this code is welcome. Most of it is just code for WinForms, but the implementation of checkForWins really feels like it can be done better.

```
using System;
using System.Collections.Generic;
using System.ComponentModel;
using System.Data;
using System.Drawing;
using System.Linq;
using System.Text;
using System.Windows.Forms;

namespace Game
{
public partial class Form1 : Form
{
Button[] buttons = new Button[16];

public Form1()
{
InitializeComponent();
grid();
MessageBox.Show("Find the pattern");
}

public void grid() {
for (int i = 0; i < buttons.Length; i++) {
buttons[i] = new Button();
}

for (int i = 0; i < 4; i++) {
buttons[i].MouseEnter += new EventHandler(HighLightm1);
buttons[i].MouseLeave += new EventHandler(unhighlightm1);
}

for (int i = 4; i < 8; i++) {
buttons[i].MouseEnter += new EventHandler(highlightm2);
buttons[i].MouseLeave += new EventHandler(unhighlightm2);
}

for (int i = 8; i < 12; i++) {
buttons[i].MouseEnter += new EventHandler(highlightm3);
buttons[i].MouseLeave += new EventHandler(unhighlightm3);
}

for (int i = 12; i < 16; i++) {
buttons[i].MouseEnter += new EventHandler(highlightm4);
butto

Naming.

public partial class Form1 : Form

Make it a habit to name things. Even MainForm or GameForm would have been nice.

Naming is hard though.

There are only two hard things in Computer Science: cache invalidation and naming things.

-- Phil Karlton

But really, it's worth even just trying to get right. Readable code loves good naming.

public void grid()

Per Method Naming Guidelines, method names in C# should be PascalCase, and should start with a verb.

Partial.

public partial class Form1 : Form

The Form1 class is partial: its definition is split between multiple files - in this case I'd wager the rest of the class is defined in the auto-generated Form1.Designer.cs.

That's where InitializeComponent is defined:

public Form1()
{
    InitializeComponent();
    //...
}

If you navigate to that .Designer.cs generated code, you'll see this:

///  
/// Required method for Designer support - do not modify 
/// the contents of this method with the code editor.
/// 
private void InitializeComponent()

The entire grid() method is the kind of code typically seen there. There's a reason a missing parameterless constructor and/or a call to InitializeComponent() in that constructor, breaks the designer: Visual Studio not only generates, but also runs this code.

By defining your buttons in the InitializeComponent() method, you effectively remove this UI boilerplate from the interesting code, and as a bonus the form designer starts looking very much like the runtime one.

I would have used the designer, and let that code be generated.

Layout.

Your layout doesn't scale. If you didn't use the designer at all, and haven't manually modified the Form1.Designer.cs file, then the user can resize the form and... totally wreck your UI.

Use the Container controls WinForms has to offer - in this case a TableLayoutPanel with fully anchored buttons would have greatly simplified the layout - here a 4x4 grid; the form's MinSize is set to 200x200:

Implementation.

This leaves us with the click handlers for the buttons, the checkForWins() method, and this:

private void Form1_Load(object sender, EventArgs e)
{
}

I don't see that handler registered anywhere. This tells me that you created the form, and then double-clicked the designer to bring up the code; that registered the Load event and added a handler stub, and now the code won't build if you remove it. Right? Use the events view in the property window to locate the form's Load event - and remove it from there. Then you can remove the handler and build.

Don't keep empty methods around. Dead code is confusing.

What's missing is an abstraction level here: your code has 16 buttons, but the problem has 4 groups of 4 buttons.

You could create a UserControl to represent a group of 4 buttons, the TableLayoutPanel can now be a 2x2 grid instead of 4x4. And it would be easier to get the proper margins, and each control would handle its own buttons.

The one can implement some model to centralize the state, and have each control (/group of 4 buttons) delegate their click handler logic to the actual form; that would be a good start IMO.

StackExchange Code Review Q#102704, answer score: 10