HalfWebDev
HalfWebDev

Reputation: 7638

Tic Tac Toe Visual basic Not comparing Button Text correctly

I am trying to code a simple Tic tac toe with 9 buttons . When i click any of buttons i am shown a message box with a WIN dialog . Though this message box is meant to show only when a WIN CASE occurs . The problem is it that it is not comparing Button.Text values correctly.

Initially All button's TEXT property is empty . On click i am setting their TEXT to "x" or "0" as the case maybe . This is suppose to work correctly but cant figure why it is not comparing the newly assigned TEXT of clicked button with other Button's Text.

Here is my code

Public Class Form1
Dim count As Int32 = 0
Private Sub Form1_Load(ByVal sender As System.Object, ByVal e As System.EventArgs) Handles MyBase.Load


End Sub
Sub checkwin()
    If Button1.Text = Button2.Text And Button2.Text = Button3.Text Then

        MessageBox.Show(Button1.Text & "wins")
        Call reset()


    ElseIf Button4.Text = Button5.Text And Button5.Text = Button6.Text Then
        MessageBox.Show(Button4.Text & "wins")
        Call reset()

    ElseIf Button7.Text = Button8.Text And Button8.Text = Button9.Text Then
        MessageBox.Show(Button7.Text & "wins")
        Call reset()
    ElseIf Button1.Text = Button4.Text And Button4.Text = Button7.Text Then
        MessageBox.Show(Button1.Text & "wins")
        Call reset()

    ElseIf Button2.Text = Button5.Text And Button5.Text = Button8.Text Then
        MessageBox.Show(Button2.Text & "wins")
        Call reset()
    ElseIf Button3.Text = Button6.Text And Button6.Text = Button9.Text Then
        MessageBox.Show(Button3.Text & "wins")
        Call reset()
    ElseIf Button1.Text = Button5.Text And Button5.Text = Button9.Text Then
        MessageBox.Show(Button1.Text & "wins")
        Call reset()

    ElseIf Button3.Text = Button5.Text And Button5.Text = Button7.Text Then
        MessageBox.Show(Button3.Text & "wins")
        Call reset()
    End If

End Sub


Sub reset()
   For Each b as Button  in Controls

        b.Text = " "
        b.Enabled = True

    Next


End Sub
Private Sub Button1_Click(ByVal sender As System.Object, ByVal e As System.EventArgs) Handles Button1.Click, Button2.Click, Button3.Click, Button4.Click, Button5.Click, Button6.Click, Button7.Click, Button8.Click, Button9.Click
    If sender Is Button1 Then
        count = count + 1
        If count Mod 2 = 0 Then
            Button1.Text = "0"

        Else
            Button1.Text = "x"
        End If

        Button1.Enabled = False
        Call checkwin()
    End If

    If sender Is Button2 Then
        count = count + 1
        If count Mod 2 = 0 Then
            Button2.Text = "0"
        Else
            Button2.Text = "x"
        End If


        Button2.Enabled = False
        Call checkwin()
    End If

    If sender Is Button3 Then
        count = count + 1
        If count Mod 2 = 0 Then
            Button3.Text = "0"
        Else
            Button3.Text = "x"
        End If

        Button3.Enabled = False
        Call checkwin()


    End If
    If sender Is Button4 Then
        count = count + 1
        If count Mod 2 = 0 Then
            Button4.Text = "0"

        Else
            Button4.Text = "x"
        End If

        Button4.Enabled = False
        Call checkwin()
    End If
    If sender Is Button5 Then
        count = count + 1
        If count Mod 2 = 0 Then
            Button5.Text = "0"

        Else
            Button5.Text = "x"
        End If

        Button5.Enabled = False
        Call checkwin()
    End If

    If sender Is Button6 Then
        count = count + 1
        If count Mod 2 = 0 Then
            Button6.Text = "0"

        Else
            Button6.Text = "x"
        End If

        Button6.Enabled = False
        Call checkwin()
    End If

    If sender Is Button7 Then
        count = count + 1
        If count Mod 2 = 0 Then
            Button7.Text = "0"

        Else
            Button7.Text = "x"
        End If

        Button7.Enabled = False
        Call checkwin()
    End If

    If sender Is Button8 Then
        count = count + 1
        If count Mod 2 = 0 Then
            Button8.Text = "0"

        Else
            Button8.Text = "x"
        End If

        Button8.Enabled = False
        Call checkwin()
    End If

    If sender Is Button9 Then
        count = count + 1
        If count Mod 2 = 0 Then
            Button9.Text = "0"

        Else
            Button9.Text = "x"
        End If

        Button9.Enabled = False
        Call checkwin()
    End If



End Sub
End Class

Upvotes: 1

Views: 1476

Answers (1)

Antagony
Antagony

Reputation: 1780

You're not eliminating unused buttons in the checkWin sub's If statements. You should also use short-circuiting comparisons to avoid unnecessary processing. So, the first If statement would look like this:

If Not Button1.Enabled AndAlso Not Button2.Enabled AndAlso Not Button3.Enabled AndAlso _
            (Button1.Text = Button2.Text) AndAlso (Button2.Text = Button3.Text) Then
    ...
ElseIf Not Button4.Enabled ... 'etc.

Oh and all those If blocks in your button click event are unnecessary. Just use the sender object, like so:

Private Sub Button1_Click(sender As System.Object, e As System.EventArgs) _
    Handles Button1.Click, Button2.Click, Button3.Click, Button4.Click, Button5.Click,
            Button6.Click, Button7.Click, Button8.Click, Button9.Click
    sender.Text = If(clickCount Mod 2 = 0, "X", "O")
    sender.Enabled = False
    clickCount += 1
    CheckWin()
End Sub

Edit (requested in comments)
To avoid the repetitive comparison code in the CheckWin sub, you can farm out the work to a function. That way the comparison code is only written once and consequently far less prone to errors and much easier to read. Something like this is what I would do:

Private Sub CheckWin()
    If LineWins(Button1, Button2, Button3) OrElse _
       LineWins(Button4, Button5, Button6) OrElse _
       LineWins(Button7, Button8, Button9) OrElse _
       LineWins(Button1, Button4, Button7) OrElse _
       LineWins(Button2, Button5, Button8) OrElse _
       LineWins(Button3, Button6, Button9) OrElse _
       LineWins(Button1, Button5, Button9) OrElse _
       LineWins(Button3, Button5, Button7) Then ResetGame()
End Sub

Private Function LineWins(b1 As Button, b2 As Button, b3 As Button) As Boolean
    LineWins = CBool(Not b1.Enabled AndAlso Not b2.Enabled AndAlso Not b3.Enabled _
                 AndAlso (b1.Text = b2.Text) AndAlso (b2.Text = b3.Text))
    If LineWins Then MessageBox.Show(b1.Text & " wins")
End Function

By the way, you may notice I've renamed "reset" to "ResetGame" – I recommend you do this as Reset is already assigned to a .NET FileSystem method.

Upvotes: 3

Related Questions