8
\$\begingroup\$

Can I optimize this Tic-tac-toe game or make it smaller?

Known optimizations: string turn = "X"; can be changed to char turn = 'x';

Form1.cs

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

namespace tictactoeAPP
{
    public partial class Form1 : Form
    {
        string turn = "X";
        byte turnno = 1;
        void Again()
        {
            DialogResult result = MessageBox.Show("Do you wish to play again?", "Again", MessageBoxButtons.YesNo);
            if (result == DialogResult.Yes)
            {
                Button[] buttons = new Button[] { button1, button2, button3, button4, button5, button6, button7, button8, button9 };
                foreach (Button item in buttons) { item.Text = ""; }
                turn = "X";
                turnno = 1;
                label1.Text = "Waiting for X";
            }
            else { this.Close(); }
        }
        void Winner()
        {
            /// 123
            /// 456
            /// 789
            /// 
            /// 159
            /// 357
            /// 
            /// 147
            /// 258
            /// 369
            /// 
            string[][] conditions = new string[][]
            {
                new string[] { button1.Text, button2.Text, button3.Text },
                new string[] { button4.Text, button5.Text, button6.Text },
                new string[] { button7.Text, button8.Text, button9.Text },
                new string[] { button1.Text, button5.Text, button9.Text },
                new string[] { button3.Text, button5.Text, button7.Text },
                new string[] { button1.Text, button4.Text, button7.Text },
                new string[] { button2.Text, button5.Text, button8.Text },
                new string[] { button3.Text, button6.Text, button9.Text }
            };
            foreach (string[] item in conditions)
            {
                if (item[0] == item[1] && item[1] == item[2] && item[0] != "") { MessageBox.Show(item[0] + " wins!", "Winner"); Again(); return; }
            }
        }
        void check_win()
        {
            Winner();
            if (turnno > 9) { MessageBox.Show("It is a tie.", "Tie"); Again(); return; }
        }
        void Assign(Button widget)
        {
            if (widget.Text == "")
            {
                turnno++;
                widget.Text = turn == "X" ? "X" : "O";
                turn = turn == "X" ? "O" : "X";
                label1.Text = "Waiting for " + turn;
                check_win();
            }
        }
        public Form1()
        {
            InitializeComponent();
        }

        private void button1_Click(object sender, EventArgs e)
        {
            Assign(button1);
        }

        private void button2_Click(object sender, EventArgs e)
        {
            Assign(button2);
        }

        private void button3_Click(object sender, EventArgs e)
        {
            Assign(button3);
        }

        private void button4_Click(object sender, EventArgs e)
        {
            Assign(button4);
        }

        private void button5_Click(object sender, EventArgs e)
        {
            Assign(button5);
        }

        private void button6_Click(object sender, EventArgs e)
        {
            Assign(button6);
        }

        private void button7_Click(object sender, EventArgs e)
        {
            Assign(button7);
        }

        private void button8_Click(object sender, EventArgs e)
        {
            Assign(button8);

        }

        private void button9_Click(object sender, EventArgs e)
        {
            Assign(button9);
        }

        private void Form1_Load(object sender, EventArgs e)
        {

        }
    }
}

Form1.Designer.cs (unchanged)

namespace tictactoeAPP
{
    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()
        {
            this.button1 = new System.Windows.Forms.Button();
            this.button2 = new System.Windows.Forms.Button();
            this.button3 = new System.Windows.Forms.Button();
            this.button4 = new System.Windows.Forms.Button();
            this.button5 = new System.Windows.Forms.Button();
            this.button6 = new System.Windows.Forms.Button();
            this.button7 = new System.Windows.Forms.Button();
            this.button8 = new System.Windows.Forms.Button();
            this.button9 = new System.Windows.Forms.Button();
            this.label1 = new System.Windows.Forms.Label();
            this.SuspendLayout();
            // 
            // button1
            // 
            this.button1.Location = new System.Drawing.Point(22, 12);
            this.button1.Name = "button1";
            this.button1.Size = new System.Drawing.Size(91, 92);
            this.button1.TabIndex = 0;
            this.button1.UseVisualStyleBackColor = true;
            this.button1.Click += new System.EventHandler(this.button1_Click);
            // 
            // button2
            // 
            this.button2.Location = new System.Drawing.Point(119, 12);
            this.button2.Name = "button2";
            this.button2.Size = new System.Drawing.Size(91, 92);
            this.button2.TabIndex = 1;
            this.button2.UseVisualStyleBackColor = true;
            this.button2.Click += new System.EventHandler(this.button2_Click);
            // 
            // button3
            // 
            this.button3.Location = new System.Drawing.Point(216, 12);
            this.button3.Name = "button3";
            this.button3.Size = new System.Drawing.Size(91, 92);
            this.button3.TabIndex = 2;
            this.button3.UseVisualStyleBackColor = true;
            this.button3.Click += new System.EventHandler(this.button3_Click);
            // 
            // button4
            // 
            this.button4.Location = new System.Drawing.Point(22, 110);
            this.button4.Name = "button4";
            this.button4.Size = new System.Drawing.Size(91, 92);
            this.button4.TabIndex = 3;
            this.button4.UseVisualStyleBackColor = true;
            this.button4.Click += new System.EventHandler(this.button4_Click);
            // 
            // button5
            // 
            this.button5.Location = new System.Drawing.Point(119, 110);
            this.button5.Name = "button5";
            this.button5.Size = new System.Drawing.Size(91, 92);
            this.button5.TabIndex = 4;
            this.button5.UseVisualStyleBackColor = true;
            this.button5.Click += new System.EventHandler(this.button5_Click);
            // 
            // button6
            // 
            this.button6.Location = new System.Drawing.Point(216, 110);
            this.button6.Name = "button6";
            this.button6.Size = new System.Drawing.Size(91, 92);
            this.button6.TabIndex = 5;
            this.button6.UseVisualStyleBackColor = true;
            this.button6.Click += new System.EventHandler(this.button6_Click);
            // 
            // button7
            // 
            this.button7.Location = new System.Drawing.Point(22, 208);
            this.button7.Name = "button7";
            this.button7.Size = new System.Drawing.Size(91, 92);
            this.button7.TabIndex = 6;
            this.button7.UseVisualStyleBackColor = true;
            this.button7.Click += new System.EventHandler(this.button7_Click);
            // 
            // button8
            // 
            this.button8.Location = new System.Drawing.Point(119, 208);
            this.button8.Name = "button8";
            this.button8.Size = new System.Drawing.Size(91, 92);
            this.button8.TabIndex = 7;
            this.button8.UseVisualStyleBackColor = true;
            this.button8.Click += new System.EventHandler(this.button8_Click);
            // 
            // button9
            // 
            this.button9.Location = new System.Drawing.Point(216, 208);
            this.button9.Name = "button9";
            this.button9.Size = new System.Drawing.Size(91, 92);
            this.button9.TabIndex = 8;
            this.button9.UseVisualStyleBackColor = true;
            this.button9.Click += new System.EventHandler(this.button9_Click);
            // 
            // label1
            // 
            this.label1.AutoSize = true;
            this.label1.Font = new System.Drawing.Font("Microsoft Sans Serif", 21.75F, System.Drawing.FontStyle.Regular, System.Drawing.GraphicsUnit.Point, ((byte)(0)));
            this.label1.Location = new System.Drawing.Point(349, 36);
            this.label1.Name = "label1";
            this.label1.Size = new System.Drawing.Size(181, 33);
            this.label1.TabIndex = 9;
            this.label1.Text = "Waiting for X";
            // 
            // Form1
            // 
            this.AutoScaleDimensions = new System.Drawing.SizeF(6F, 13F);
            this.AutoScaleMode = System.Windows.Forms.AutoScaleMode.Font;
            this.ClientSize = new System.Drawing.Size(691, 314);
            this.Controls.Add(this.label1);
            this.Controls.Add(this.button9);
            this.Controls.Add(this.button8);
            this.Controls.Add(this.button7);
            this.Controls.Add(this.button6);
            this.Controls.Add(this.button5);
            this.Controls.Add(this.button4);
            this.Controls.Add(this.button3);
            this.Controls.Add(this.button2);
            this.Controls.Add(this.button1);
            this.Name = "Form1";
            this.Text = "Form1";
            this.Load += new System.EventHandler(this.Form1_Load);
            this.ResumeLayout(false);
            this.PerformLayout();

        }

        #endregion

        private System.Windows.Forms.Button button1;
        private System.Windows.Forms.Button button2;
        private System.Windows.Forms.Button button3;
        private System.Windows.Forms.Button button4;
        private System.Windows.Forms.Button button5;
        private System.Windows.Forms.Button button6;
        private System.Windows.Forms.Button button7;
        private System.Windows.Forms.Button button8;
        private System.Windows.Forms.Button button9;
        private System.Windows.Forms.Label label1;
    }
}

Program.cs (unchanged)

using System;
using System.Collections.Generic;
using System.Linq;
using System.Threading.Tasks;
using System.Windows.Forms;

namespace tictactoeAPP
{
    static class Program
    {
        /// <summary>
        /// The main entry point for the application.
        /// </summary>
        [STAThread]
        static void Main()
        {
            Application.EnableVisualStyles();
            Application.SetCompatibleTextRenderingDefault(false);
            Application.Run(new Form1());
        }
    }
}
\$\endgroup\$
2
  • \$\begingroup\$ SInce you're asking about making it smaller, are you not interested in answers that focus on improving readability if it adds code rather than remove it? (e.g. abstraction of logic into classes). \$\endgroup\$
    – Flater
    Commented Jun 15, 2018 at 6:38
  • \$\begingroup\$ Anything that improves it is appreciated. \$\endgroup\$
    – Omar.B
    Commented Jun 15, 2018 at 8:36

2 Answers 2

4
\$\begingroup\$

There is not much to say about the code. It works as expected as a simple tic-tac-toe-game, and you show good understanding of how to code in C#.

One can discuss if a line like this:

if (item[0] == item[1] && item[1] == item[2] && item[0] != "") { MessageBox.Show(item[0] + " wins!", "Winner"); Again(); return; }

is good or bad or best practice. I find it hard to read because I'm used to have statements line by line as:

if (item[0] == item[1] && item[1] == item[2] && item[0] != "") 
{ 
  MessageBox.Show(item[0] + " wins!", "Winner"); 
  Again(); 
  return; 
}

According to optimization, I don't think that should be your major concern (there is really nothing to optimize). Instead I would focus on how to separate game logic from the UI. As for now, your code is relying solely on the state of the UI-controls, and that is commonly regarded as bad design, because you then are bound to a specific UI (WinForms). Instead you should build a Game model, that can keep track of the game state. As a template for that you could do something like:

public class TTTGame
{
  Field[,] _fields;

  TTTGame()
  {
    // TODO: Initialize fields
  }

  public void SetField(Player player, int row, col)
  {
    _fields[row, col].Player = player;
  }

  public State GetState()
  {
     // TODO: check _fields to see if there is a winning "row" or if the game is over with a tie.
    return new State(/* TODO with properties */);
  }      
}

public class Field 
{
  public Player Player { get; set; }
}

public enum Player
{
  None,
  X,
  O
}

public class State
{
   // TODO implement what ever properties are needed to describe the current state (winning player, game over, tie etc.
}

The above may not be the best/state of the art solution, but is just meant as inspiration.


The next challenge would be to implement Player classes - a human player and a an "AI"-player, so you would be able to play against the "computer".

\$\endgroup\$
2
  • 2
    \$\begingroup\$ +1, separation of logic is the next step here. One remark though: public enum Player I'd suggest calling it Token. I know that in TTT, a player and token are uniquely bound to each other; but it makes more linguistic sense for a Field to contain a Token, and a Player to have an assigned Token value. \$\endgroup\$
    – Flater
    Commented Jun 15, 2018 at 9:09
  • \$\begingroup\$ @Flater: that'll make sense too. \$\endgroup\$
    – user73941
    Commented Jun 15, 2018 at 9:24
1
\$\begingroup\$

I would have a Button class that assigns text.

You can have a single click event handler. object sender will be the button. Just have to cast it to button.

Disable the button so they can not even click it once it is selected. I think you can do it with CanSelect.

I would like to see a button class that does more of heavy lifting:

 public xp XPturn { get; private set; }
 public void FlipXPturn();
 {
     if (XP == xp.X)
     {
         XP = xp.O;
     }
     else 
     {
         XP = xp.X;
     }
 }
 public enum xp { X, O}
 public class Button 
 {  
      public xp XP { get; private set; }
      public string Text { get { return }; }
      public bool IsSelected { get; private set; } = false;
      public void Click()
      {
           IsSelected = true;
           XP = XPturn;
           FlipXPturn();
      }
      public void Clear() 
      {
           IsSelected = false;
           XP = null;
      }
 }
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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