7
\$\begingroup\$

I made a minesweeper game in c# as a method of studying, and right now its fully operational.

I tried to use inheritance, recursion, and other fundamentals as clean as possible. Also I tried to use commenting as much as possible as I see many people do not do this, and it makes the code all the more confusing, both for the reader and the coder (at least for me).

I tried my best to handle exceptions and anticipate possible problems, and I fixed any I could find. There was one exception however, it was a stack overflow exception, but it happened only one time and i couldn't reproduce it again to correct. If anyone can reproduce it and inform why and how, it will be much appreciated.

If some part of code is missing, just let me know and i will add them as an edit.

Thank you very much for your help.

Form1.cs :

using MineSweeper.Properties;
using System;
using System.Collections.Generic;
using System.Drawing;
using System.Windows.Forms;

namespace MineSweeper
{
    public partial class Form1 : Form
    {
        public Form1()
        {
            InitializeComponent();
        }

        readonly string msgBoxTitle = "Warning";
        List<int> panels = new List<int>();
        int rowCount;
        int columnCount;

        private void ModifyMineField()
        {
            panels.Clear(); //clears the array in case user will make a new field
            MineFieldTable.RowStyles.Clear(); //resets the table
            MineFieldTable.ColumnStyles.Clear(); //resets the table
            MineFieldTable.Controls.Clear(); //resets the table
            MineFieldTable.Visible = false;
            rowCount = Convert.ToInt32(TB_tableDimension1.Text); //take rowCount from user input
            columnCount = Convert.ToInt32(TB_tableDimension2.Text); //take columnCount from user input
            int mineCount = Convert.ToInt32(TB_mineCount.Text); //amount of mines, decided by user
            MineFieldTable.CellBorderStyle = TableLayoutPanelCellBorderStyle.Single;
            MineFieldTable.AutoSize = true;
            MineFieldTable.RowCount = rowCount;
            MineFieldTable.ColumnCount = columnCount;
            Random rnd = new Random();
            int counter = 0;
            int[] panelArray = new int[rowCount * columnCount]; //track the panel layout, 0= empty, 1=mine, 2=opened

            for (int i = 0; i < panelArray.Length; i++) //populate the array with empty land, to be re-designated later
            {
                panelArray[i] = 0;
            }

            while (counter < mineCount) //creating mine coordinates by array numbering
            {
                int a = rnd.Next(0, rowCount * columnCount - 1);
                if (panelArray[a] != 1) { panelArray[a] = 1; counter++; }
            }

            panels.AddRange(panelArray);

            for (int i = 0; i < MineFieldTable.RowCount; i++) //generating row styles
            {
                MineFieldTable.RowStyles.Add(new RowStyle(SizeType.Absolute, 30f));
            }

            for (int i = 0; i < MineFieldTable.ColumnCount; i++) //generating column styles
            {
                MineFieldTable.ColumnStyles.Add(new ColumnStyle(SizeType.Absolute, 30f));
            }

            for (int i = 0; i < rowCount * columnCount; i++) //lay the field with mines or not
            {
                if (panels[i] != 1) //add empty land
                {
                    NoFocusButton emptyLand = new NoFocusButton("", FlatStyle.Standard, 0) { };
                    emptyLand.MouseUp += Land_Click;
                    MineFieldTable.Controls.Add(emptyLand);
                }

                if (panels[i] == 1) //add mines
                {
                    NoFocusButton mine = new NoFocusButton("", FlatStyle.Standard, 1) { };
                    mine.MouseUp += Land_Click;
                    MineFieldTable.Controls.Add(mine);
                }
            }
            MineFieldTable.Visible = true; //to make field generation quicker, first invisible, then visible
        }

        private void BtnCreate_Click(object sender, EventArgs e)
        {
            if ( //check if user entered correct parameters
                int.TryParse(TB_mineCount.Text, out _) == false
                || int.TryParse(TB_tableDimension1.Text, out _) == false
                || int.TryParse(TB_tableDimension2.Text, out _) == false
                || Convert.ToInt32(TB_mineCount.Text) < 1
                || Convert.ToInt32(TB_tableDimension1.Text) < 1
                || Convert.ToInt32(TB_tableDimension1.Text) > 30
                || Convert.ToInt32(TB_tableDimension2.Text) < 1
                || Convert.ToInt32(TB_tableDimension2.Text) > 30
                || Convert.ToInt32(TB_mineCount.Text) > Convert.ToInt32(TB_tableDimension1.Text) * Convert.ToInt32(TB_tableDimension2.Text))
            {
                MessageBox.Show("Please enter valid values, and consider the following: \n \n-Field cannot be bigger than 30x30.\n \n-Mines cannot be more than the field size.", msgBoxTitle);
            }
            else
            {
                try { MineFieldTable.Visible = false; ModifyMineField(); } catch (Exception ex) { MessageBox.Show(ex.Message, msgBoxTitle); }
            }
        }

        private void Land_Click(object sender, MouseEventArgs e)
        {
            TableLayoutPanelCellPosition clickCoords = MineFieldTable.GetPositionFromControl((Control)sender); //obtaining coordinates of control
            NoFocusButton currentControl = MineFieldTable.GetControlFromPosition(clickCoords.Column, clickCoords.Row) as NoFocusButton; //the current control, obtained from coords

            if (e.Button == MouseButtons.Left & currentControl.Text != ".")
            {
                if (panels[clickCoords.Column + (clickCoords.Row) * columnCount] == 0) //if the clicked land is empty land
                {
                    currentControl.LandOpening(clickCoords.Row, clickCoords.Column, MineFieldTable, panels);
                }

                if (panels[clickCoords.Column + (clickCoords.Row) * columnCount] == 1) //stepped on a mine!
                {
                    NoFocusButton currentButton = MineFieldTable.GetControlFromPosition(clickCoords.Column, clickCoords.Row) as NoFocusButton;
                    currentButton.BackgroundImage = Resources.mineExplode;
                    currentButton.Text = "";
                    currentButton.FlatStyle = FlatStyle.Flat;
                    NoFocusButton currentButton2; //other panels on the land other than the stepped panel

                    for (int i = 0; i < panels.Count; i++)
                    {
                        if (panels[i] == 1)
                        {
                            int row = i / columnCount;
                            int column = i % columnCount;
                            currentButton2 = MineFieldTable.GetControlFromPosition(column, row) as NoFocusButton;

                            if (clickCoords.Row != row | clickCoords.Column != column) //in order to not replace the red mine
                            {
                                currentButton2.FlatStyle = FlatStyle.Flat;
                                currentButton2.BackgroundImage = Resources.mine;
                            }

                            if (currentButton2.Text == ".") //if mine was flagged, show it as green
                            {
                                currentButton2.FlatStyle = FlatStyle.Flat;
                                currentButton2.Image = null;
                                currentButton2.BackgroundImage = Resources.mineGreen;
                            }
                        }

                        if (panels[i] != 1)
                        {
                            int row = i / columnCount;
                            int column = i % columnCount;
                            currentButton2 = MineFieldTable.GetControlFromPosition(column, row) as NoFocusButton;

                            if (currentButton2.Text == ".") //if a flag was placed wrong, indicate that it was wrong
                            {
                                currentButton2.Image = null;
                                currentButton2.Text = "";
                                currentButton2.Image = Resources.flag2crossed;
                            }
                        }
                    }
                    MessageBox.Show("You lost the game!", "Game Over");
                    MineFieldTable.Visible = false;
                    MineFieldTable.Controls.Clear();
                }
            }

            if (e.Button == MouseButtons.Right) //flagging land where suspected to be mines
            {
                if (currentControl.Text == "") //place flag
                {
                    currentControl.Image = Resources.flag2;
                    currentControl.Text = ".";
                    currentControl.ForeColor = Color.Gray;
                }
                else if (currentControl.Text == ".") //remove flag
                {
                    currentControl.Image = null;
                    currentControl.Text = "";
                }
            }

            if (panels.Contains(0) == false) //if all mines are revealed, finish the game
            {
                for (int i = 0; i < panels.Count; i++)
                {
                    if (panels[i] == 1)
                    {
                        int row = i / columnCount;
                        int column = i % columnCount;

                        NoFocusButton currentButton2 = MineFieldTable.GetControlFromPosition(column, row) as NoFocusButton;

                        if (currentButton2.Text == ".") //if mine was flagged, show it as green
                        {
                            currentButton2.FlatStyle = FlatStyle.Flat;
                            currentButton2.Image = null;
                            currentButton2.BackgroundImage = Resources.mineGreen;
                        }

                        if (currentButton2.Text != ".") //if mine was not flagged, show as usual
                        {
                            currentButton2.FlatStyle = FlatStyle.Flat;
                            currentButton2.BackgroundImage = Resources.mine;
                        }
                    }
                }
                MessageBox.Show("You won the game!", "Game Over");
                MineFieldTable.Visible = false;
                MineFieldTable.Controls.Clear();
            }
        }
    }
}

NoFocusButton.cs (a custom class for buttons) :

using MineSweeper.Properties;
using System;
using System.Collections.Generic;
using System.Drawing;
using System.Windows.Forms;

namespace MineSweeper
{
    class NoFocusButton : Button
    {

        public NoFocusButton(string text, FlatStyle flatstyle, int indexnumber)
        {
            SetStyle(ControlStyles.Selectable, false);
            Text = text;
            TextAlign = ContentAlignment.MiddleCenter;
            Margin = new Padding(0);
            Padding = new Padding(0);
            Dock = DockStyle.Fill;
            FlatStyle = flatstyle;
            FlatAppearance.BorderSize = 0;
        }

        public void LandOpening(int coordinateX, int coordinateY, TableLayoutPanel table, List<int> panelArray) //if the opened land is empty, do this
        {
            for (int i = -1; i < 2; i++)
            {
                for (int j = -1; j < 2; j++)
                {
                    try
                    {
                        int a = coordinateY + j; int b = coordinateX + i; //coordinates of the current control

                        if (a > -1 && b > -1 && a < table.ColumnCount && b < table.RowCount) //to confirm if the control index is not out of bounds
                        {
                            NoFocusButton currentButton = table.GetControlFromPosition(a, b) as NoFocusButton;

                            if (panelArray[a + b * table.ColumnCount] == 0) //if the nearby land is empty, write the mine count on that land
                            {
                                int nearbyMines = 0;

                                for (int k = -1; k < 2; k++) //to determine minecount near opened land and write it
                                {
                                    for (int l = -1; l < 2; l++)
                                    {
                                        int d = coordinateY + j + k; int e = coordinateX + i + l; //to shorten future code

                                        if (d > -1 && e > -1 && d < table.ColumnCount && e < table.RowCount) //to confirm if the control index is not out of bounds
                                        {
                                            NoFocusButton openingButton = table.GetControlFromPosition(d, e) as NoFocusButton;
                                            bool c = int.TryParse(openingButton.Text, out int n); //to shorten future code

                                            if (panelArray[d + e * table.ColumnCount] == 1 && c == false) //to check if cell has number inside
                                            {
                                                nearbyMines++;
                                            }
                                        }
                                    }
                                }

                                if (a >= 0 && b >= 0 && a <= table.ColumnCount && b <= table.RowCount) //to confirm if the control index is not out of bounds
                                {
                                    NoFocusButton x = table.GetControlFromPosition(a, b) as NoFocusButton;

                                    if (nearbyMines != 0 & x.Text !=".") //dont write 0 mines
                                    {
                                        x.Text = nearbyMines.ToString();
                                    }

                                    if (x.Text != ".")
                                    {
                                        x.FlatStyle = FlatStyle.Flat;
                                        x.Enabled = false;
                                        panelArray[a + b * table.ColumnCount] = 2; //mark this panel in the array as opened
                                    }

                                }

                                if (nearbyMines == 0) //using recursion to open more area if there is a panel with zero at the border of opened area
                                {
                                    LandOpening(b, a, table, panelArray);
                                }
                            }
                        }
                    }
                    catch (Exception ex) { MessageBox.Show(ex.Message); }
                }
            }
        }
    }
}

Form1.Designer.cs :

namespace MineSweeper
{
    partial class Form1
    {
        /// <summary>
        /// Required designer variable.
        /// </summary>
        private System.ComponentModel.IContainer components = null;

        /// <summary>
        /// Clean up any resources being used.
        /// </summary>
        /// <param name="disposing">true if managed resources should be disposed; otherwise, false.</param>
        protected override void Dispose(bool disposing)
        {
            if (disposing && (components != null))
            {
                components.Dispose();
            }
            base.Dispose(disposing);
        }

        #region Windows Form Designer generated code

        /// <summary>
        /// Required method for Designer support - do not modify
        /// the contents of this method with the code editor.
        /// </summary>
        private void InitializeComponent()
        {
            System.ComponentModel.ComponentResourceManager resources = new System.ComponentModel.ComponentResourceManager(typeof(Form1));
            this.TB_tableDimension1 = new System.Windows.Forms.TextBox();
            this.TB_tableDimension2 = new System.Windows.Forms.TextBox();
            this.BtnCreate = new System.Windows.Forms.Button();
            this.MineFieldTable = new System.Windows.Forms.TableLayoutPanel();
            this.TB_mineCount = new System.Windows.Forms.TextBox();
            this.label2 = new System.Windows.Forms.Label();
            this.label3 = new System.Windows.Forms.Label();
            this.label1 = new System.Windows.Forms.Label();
            this.SuspendLayout();
            // 
            // TB_tableDimension1
            // 
            this.TB_tableDimension1.Location = new System.Drawing.Point(20, 25);
            this.TB_tableDimension1.MaxLength = 2;
            this.TB_tableDimension1.Name = "TB_tableDimension1";
            this.TB_tableDimension1.Size = new System.Drawing.Size(20, 20);
            this.TB_tableDimension1.TabIndex = 2;
            this.TB_tableDimension1.Text = "10";
            // 
            // TB_tableDimension2
            // 
            this.TB_tableDimension2.Location = new System.Drawing.Point(50, 25);
            this.TB_tableDimension2.MaxLength = 2;
            this.TB_tableDimension2.Name = "TB_tableDimension2";
            this.TB_tableDimension2.Size = new System.Drawing.Size(20, 20);
            this.TB_tableDimension2.TabIndex = 3;
            this.TB_tableDimension2.Text = "10";
            // 
            // BtnCreate
            // 
            this.BtnCreate.Location = new System.Drawing.Point(160, 25);
            this.BtnCreate.Name = "BtnCreate";
            this.BtnCreate.Size = new System.Drawing.Size(75, 20);
            this.BtnCreate.TabIndex = 1;
            this.BtnCreate.Text = "CREATE";
            this.BtnCreate.UseVisualStyleBackColor = true;
            this.BtnCreate.Click += new System.EventHandler(this.BtnCreate_Click);
            // 
            // MineFieldTable
            // 
            this.MineFieldTable.AutoSizeMode = System.Windows.Forms.AutoSizeMode.GrowAndShrink;
            this.MineFieldTable.CellBorderStyle = System.Windows.Forms.TableLayoutPanelCellBorderStyle.Single;
            this.MineFieldTable.ColumnCount = 10;
            this.MineFieldTable.ColumnStyles.Add(new System.Windows.Forms.ColumnStyle(System.Windows.Forms.SizeType.Absolute, 20F));
            this.MineFieldTable.ColumnStyles.Add(new System.Windows.Forms.ColumnStyle(System.Windows.Forms.SizeType.Absolute, 20F));
            this.MineFieldTable.ColumnStyles.Add(new System.Windows.Forms.ColumnStyle(System.Windows.Forms.SizeType.Absolute, 20F));
            this.MineFieldTable.ColumnStyles.Add(new System.Windows.Forms.ColumnStyle(System.Windows.Forms.SizeType.Absolute, 20F));
            this.MineFieldTable.ColumnStyles.Add(new System.Windows.Forms.ColumnStyle(System.Windows.Forms.SizeType.Absolute, 20F));
            this.MineFieldTable.ColumnStyles.Add(new System.Windows.Forms.ColumnStyle(System.Windows.Forms.SizeType.Absolute, 20F));
            this.MineFieldTable.ColumnStyles.Add(new System.Windows.Forms.ColumnStyle(System.Windows.Forms.SizeType.Absolute, 20F));
            this.MineFieldTable.ColumnStyles.Add(new System.Windows.Forms.ColumnStyle(System.Windows.Forms.SizeType.Absolute, 20F));
            this.MineFieldTable.ColumnStyles.Add(new System.Windows.Forms.ColumnStyle(System.Windows.Forms.SizeType.Absolute, 20F));
            this.MineFieldTable.ColumnStyles.Add(new System.Windows.Forms.ColumnStyle(System.Windows.Forms.SizeType.Absolute, 30F));
            this.MineFieldTable.Location = new System.Drawing.Point(20, 50);
            this.MineFieldTable.Margin = new System.Windows.Forms.Padding(0);
            this.MineFieldTable.Name = "MineFieldTable";
            this.MineFieldTable.RowCount = 10;
            this.MineFieldTable.RowStyles.Add(new System.Windows.Forms.RowStyle(System.Windows.Forms.SizeType.Absolute, 20F));
            this.MineFieldTable.RowStyles.Add(new System.Windows.Forms.RowStyle(System.Windows.Forms.SizeType.Absolute, 20F));
            this.MineFieldTable.RowStyles.Add(new System.Windows.Forms.RowStyle(System.Windows.Forms.SizeType.Absolute, 20F));
            this.MineFieldTable.RowStyles.Add(new System.Windows.Forms.RowStyle(System.Windows.Forms.SizeType.Absolute, 20F));
            this.MineFieldTable.RowStyles.Add(new System.Windows.Forms.RowStyle(System.Windows.Forms.SizeType.Absolute, 20F));
            this.MineFieldTable.RowStyles.Add(new System.Windows.Forms.RowStyle(System.Windows.Forms.SizeType.Absolute, 20F));
            this.MineFieldTable.RowStyles.Add(new System.Windows.Forms.RowStyle(System.Windows.Forms.SizeType.Absolute, 20F));
            this.MineFieldTable.RowStyles.Add(new System.Windows.Forms.RowStyle(System.Windows.Forms.SizeType.Absolute, 20F));
            this.MineFieldTable.RowStyles.Add(new System.Windows.Forms.RowStyle(System.Windows.Forms.SizeType.Absolute, 20F));
            this.MineFieldTable.RowStyles.Add(new System.Windows.Forms.RowStyle(System.Windows.Forms.SizeType.Absolute, 20F));
            this.MineFieldTable.Size = new System.Drawing.Size(210, 210);
            this.MineFieldTable.TabIndex = 4;
            this.MineFieldTable.Visible = false;
            // 
            // TB_mineCount
            // 
            this.TB_mineCount.Location = new System.Drawing.Point(100, 25);
            this.TB_mineCount.MaxLength = 3;
            this.TB_mineCount.Name = "TB_mineCount";
            this.TB_mineCount.Size = new System.Drawing.Size(25, 20);
            this.TB_mineCount.TabIndex = 4;
            this.TB_mineCount.Text = "10";
            // 
            // label2
            // 
            this.label2.AutoSize = true;
            this.label2.Location = new System.Drawing.Point(70, 28);
            this.label2.Name = "label2";
            this.label2.Size = new System.Drawing.Size(27, 13);
            this.label2.TabIndex = 6;
            this.label2.Text = "Size";
            // 
            // label3
            // 
            this.label3.AutoSize = true;
            this.label3.Location = new System.Drawing.Point(125, 28);
            this.label3.Name = "label3";
            this.label3.Size = new System.Drawing.Size(35, 13);
            this.label3.TabIndex = 7;
            this.label3.Text = "Mines";
            // 
            // label1
            // 
            this.label1.AutoSize = true;
            this.label1.Font = new System.Drawing.Font("Microsoft Sans Serif", 6F, System.Drawing.FontStyle.Regular, System.Drawing.GraphicsUnit.Point, ((byte)(0)));
            this.label1.Location = new System.Drawing.Point(40, 31);
            this.label1.Name = "label1";
            this.label1.Size = new System.Drawing.Size(10, 9);
            this.label1.TabIndex = 8;
            this.label1.Text = "X";
            // 
            // Form1
            // 
            this.AutoScaleDimensions = new System.Drawing.SizeF(6F, 13F);
            this.AutoScaleMode = System.Windows.Forms.AutoScaleMode.Font;
            this.AutoScroll = true;
            this.AutoSize = true;
            this.ClientSize = new System.Drawing.Size(284, 261);
            this.Controls.Add(this.label1);
            this.Controls.Add(this.label3);
            this.Controls.Add(this.label2);
            this.Controls.Add(this.TB_mineCount);
            this.Controls.Add(this.MineFieldTable);
            this.Controls.Add(this.BtnCreate);
            this.Controls.Add(this.TB_tableDimension2);
            this.Controls.Add(this.TB_tableDimension1);
            this.FormBorderStyle = System.Windows.Forms.FormBorderStyle.FixedDialog;
            this.Icon = ((System.Drawing.Icon)(resources.GetObject("$this.Icon")));
            this.MaximizeBox = false;
            this.MinimizeBox = false;
            this.Name = "Form1";
            this.Padding = new System.Windows.Forms.Padding(0, 0, 20, 20);
            this.Text = "MineSweeper by MHS";
            this.ResumeLayout(false);
            this.PerformLayout();

        }

        #endregion
        private System.Windows.Forms.TextBox TB_tableDimension1;
        private System.Windows.Forms.TextBox TB_tableDimension2;
        private System.Windows.Forms.Button BtnCreate;
        private System.Windows.Forms.TableLayoutPanel MineFieldTable;
        private System.Windows.Forms.TextBox TB_mineCount;
        private System.Windows.Forms.Label label2;
        private System.Windows.Forms.Label label3;
        private System.Windows.Forms.Label label1;
    }
}
\$\endgroup\$
  • 2
    \$\begingroup\$ You should have a board class that represents the logical board, which is (mostly) an multidimensional array of panels. Don't mix you game logic with your UI controls. You should be able to lift your "board" class and game logic out and use it without any modifications in a MVC web application, or winforms, or WPF, etc. \$\endgroup\$ – Robert McKee Nov 25 '19 at 19:28
  • 1
    \$\begingroup\$ Then you can either have each panel have a list of neighbors precalculated at board creation time (quickest at runtime, but more slightly more memory), or have the board class have a method that will return a list of neighbors for any given panel. \$\endgroup\$ – Robert McKee Nov 25 '19 at 19:37
  • \$\begingroup\$ It's very impressive you were able to get this to work with brute force. Implementing correct minesweeper behavior is a deceptively difficult task. My main suggestion to you is the separate all the "engine" logic from the UI. To see what I mean, take a look at my minesweeper library which excludes the UI, but does include a suite of tests. github.com/bradmarder/MSEngine \$\endgroup\$ – Brad M Nov 26 '19 at 14:36
5
\$\begingroup\$

This isn't object-oriented code. In fact the only difference between this implementation and a VBA macro popping up a modal UserForm that completely runs the show, is the language and framework involved: this anti-pattern has a name, and that's "Smart UI".

The biggest problem with it, is that the entire game logic is completely intertwined with UI concerns, which means none of the game logic can be unit-tested.


Form1.Designer.cs

Being a designer-generated code file, none of this code should be manually edited, and could have been omitted. The nice thing about including it, is that it becomes reviewable code, because we get to see what you've named all the controls.

I like that you've given a meaningful name to the MineFieldTable layout panel, but these also deserve a meaningful name:

this.Name = "Form1";
private System.Windows.Forms.TextBox TB_tableDimension1;
private System.Windows.Forms.TextBox TB_tableDimension2;
private System.Windows.Forms.Label label2;
private System.Windows.Forms.Label label3;
private System.Windows.Forms.Label label1;

The last thing you want to see in your form's code-behind, is stuff like label2 and TB_tableDimension1 - it says nothing about what the purpose of label2 is, and leaves the reader assuming. You seem to want to use some Hungarian Notation prefixing scheme (Button -> Btn; TextBox -> TB_), but the scheme is not consistent (where's the type prefix for the MineFieldTable? Why does TB_ have an underscore, but not Btn? Why is TB all-caps, but Btn is PascalCase?)... make your life simpler and drop this prefixing habit.

Consider using a fully spelled-out postfix instead:

  • TableHeightBox vs TB_tableDimension1
  • TableWidthBox vs TB_tableDimension2
  • CreateButton vs BtnCreate
  • MineFieldPanel vs MineFieldTable
  • MineCountBox vs TB_mineCount
  • MinesLabel vs label3
  • GridSizeLabel vs label2
  • WhateverThatIsLabel vs label1 (why is there a label with just an "X" in it anyway?)

Form1.cs

As mentioned above, the form is doing way, way too many things. In object-oriented code, I would expect the form's code-behind to interact with the game state. Here, the form's controls are the game state.

You will want to heavily refactor everything in here, and pull most of the logic into other classes - and to achieve this, you'll need to actually model the game state, so that the code that's responsible for evaluating game state doesn't need to care for buttons and textboxes.

The first thing I would expect in a Minesweeper-type game, is a class that respresents a grid position and its state.

This is your game state:

int[] panelArray = new int[rowCount * columnCount]; //track the panel layout, 0= empty, 1=mine, 2=opened

..but it's local to the method creating the controls/grid, nothing else can access it!

You see the problem with intertwined game and UI logic, is that...

    if (currentButton2.Text != ".") //if mine was not flagged, show as usual
    {
        currentButton2.FlatStyle = FlatStyle.Flat;
        currentButton2.BackgroundImage = Resources.mine;
    }

...game state should be pulled from your model, not from the controls on the form!


NoFocusButton.cs

These nested loops are very scary, and the single-character loop variable names make them very hard to follow. LandOpening is a poor name for such an important method - you want method names that start with a verb, and that describe what they do / what their purpose is. The domain-specifics aren't clear. What's a "land" and why does it "open"?

You want the game state to be well-defined enum values, not hard-coded 1/0/-1 integer literals; you want the underlying value of these enum states to be irrelevant (so, no looping from -1 to <2); you want meaningful names for every identifier: i, j, k, a, d, ...there's just too many of them - note that the lowercase L/l is the most evil of all (looks way too much like a 1 at a glance).

Rule of thumb, if you're looking at a 4-level nested loop structure with conditionals and recursive calls, you're in a bad place called Spaghetti Land - that elusive stack overflow exception is definitely caused by the recursive logic you have there.

Recursion and loops are, most often, different tools for solving the same problem: recursive logic can be expressed with a loop, and a loop can be expressed with recursive logic. Mixing the two makes the code a mind-bender that becomes very hard to follow.


Object Oriented

As Robert mentioned, the first thing to do when you want to write object-oriented code isn't to think of a UI, but of how you're going to model the game components.

You want a GameBoard (or MineField) object responsible for encapsulating the game state; you want that object to have methods like Clear and Initialize(int width, int height, int mines), you want a MineFieldCell object that can hold CellState and VisibleState values - running the Initialize method would create that width*height MineFieldCell objects with VisibleState = VisibleState.Masked and CellState = CellState.Mine or CellState = CellState.Safe, so you'd need an enum CellState type with Mine and Safe (or NoMine.. whatever), and an enum VisibleState type with Masked, Flagged and Exposed values; you'll want a MineFieldCell to expose an integer property representing the number of adjacent mines that the UI will read from when the VisibleState is VisibleState.Exposed (can be computed up-front, on on-the-fly)... and then you'll want unit tests that validate that all this logic works as intended - and all of that can (and should) happen before you wire any of it up to any UI.

| improve this answer | |
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.