1

I'm writing a Visual Basic program to emulate a Tic Tac Toe game. I have a method that, to my knowledge, should correctly draw the board and fill it with Xs and Os appropriately. However when I test it, it will only draw them in the same row.

"Sub board" draws everything, testing for existing values of "x" and "o" and respectively calls "drawX" or "drawO", passing the coordinates to draw.

Sub Main()
    Dim start As String = String.Empty
    Dim x As ArrayList = New ArrayList
    Dim o As ArrayList = New ArrayList
    Dim choice As String = String.Empty
    o.Add("1")
    o.Add("5")
    o.Add("9")

    While True
        board(x, o)
        pause()
    End While
End Sub

Sub pause()
    Console.WriteLine("Press enter to continue...")
    Console.ReadLine()
End Sub

Sub board(x As ArrayList, o As ArrayList)
    Console.ForegroundColor = 8
    Console.WriteLine("                  **                  **                  ")
    Console.WriteLine("                  **                  **                  ")
    Console.WriteLine("                  **                  **                  ")
    Console.WriteLine("                  **                  **                  ")
    Console.WriteLine("                  **                  **                  ")
    Console.WriteLine("                  **                  **                  ")
    Console.WriteLine("                  **                  **                  ")
    Console.WriteLine("                  **                  **                  ")
    Console.WriteLine("                  **                  **                  ")
    Console.WriteLine("**********************************************************")
    Console.WriteLine("**********************************************************")
    Console.WriteLine("                  **                  **                  ")
    Console.WriteLine("                  **                  **                  ")
    Console.WriteLine("                  **                  **                  ")
    Console.WriteLine("                  **                  **                  ")
    Console.WriteLine("                  **                  **                  ")
    Console.WriteLine("                  **                  **                  ")
    Console.WriteLine("                  **                  **                  ")
    Console.WriteLine("                  **                  **                  ")
    Console.WriteLine("                  **                  **                  ")
    Console.WriteLine("**********************************************************")
    Console.WriteLine("**********************************************************")
    Console.WriteLine("                  **                  **                  ")
    Console.WriteLine("                  **                  **                  ")
    Console.WriteLine("                  **                  **                  ")
    Console.WriteLine("                  **                  **                  ")
    Console.WriteLine("                  **                  **                  ")
    Console.WriteLine("                  **                  **                  ")
    Console.WriteLine("                  **                  **                  ")
    Console.WriteLine("                  **                  **                  ")
    Console.WriteLine("                  **                  **                  ")

    Dim ver As Integer = 1
    Dim hor As Integer = 4
    Dim vSpace As Integer = 11
    Dim hSpace As Integer = 20
    Dim vPos As Integer = 0
    Dim hPos As Integer = 0
    For i As Integer = 1 To 9
        If 1 <= i <= 3 Then
            vPos = ver
            Select Case i
                Case 1
                    hPos = hor
                Case 2
                    hPos = hor + hSpace
                Case 3
                    hPos = hor + (hSpace * 2)
                Case Else
            End Select
        End If
        If 4 <= i <= 6 Then
            vPos = ver + vSpace
            Select Case i
                Case 4
                    hPos = hor
                Case 5
                    hPos = hor + hSpace
                Case 6
                    hPos = hor + (hSpace * 2)
                Case Else
            End Select
        End If
        If 7 <= i <= 9 Then
            vPos = ver + (vSpace * 2)
            Select Case i
                Case 7
                    hPos = hor
                Case 8
                    hPos = hor + hSpace
                Case 9
                    hPos = hor + (hSpace * 2)
                Case Else
            End Select
        End If
        Select Case True
            Case x.Contains(CStr(i))
                drawX(hPos, vPos)
            Case o.Contains(CStr(i))
                drawO(hPos, vPos)
        End Select
    Next i
End Sub

Sub drawX(hPos As Integer, vPos As Integer)
    Console.ForegroundColor = 6
    Console.SetCursorPosition(hPos, vPos)
    Console.Write("**      **")
    Console.SetCursorPosition(hPos, vPos + 1)
    Console.Write(" **    **")
    Console.SetCursorPosition(hPos, vPos + 2)
    Console.Write("  **  **")
    Console.SetCursorPosition(hPos, vPos + 3)
    Console.Write("   ****")
    Console.SetCursorPosition(hPos, vPos + 4)
    Console.Write("  **  **")
    Console.SetCursorPosition(hPos, vPos + 5)
    Console.Write(" **    **")
    Console.SetCursorPosition(hPos, vPos + 6)
    Console.Write("**      **")

End Sub

Sub drawO(hPos As Integer, vPos As Integer)
    Console.ForegroundColor = 3
    Console.SetCursorPosition(hPos, vPos)
    Console.Write("  ******")
    Console.SetCursorPosition(hPos, vPos + 1)
    Console.Write(" ********")
    Console.SetCursorPosition(hPos, vPos + 2)
    Console.Write("**      **")
    Console.SetCursorPosition(hPos, vPos + 3)
    Console.Write("**      **")
    Console.SetCursorPosition(hPos, vPos + 4)
    Console.Write("**      **")
    Console.SetCursorPosition(hPos, vPos + 5)
    Console.Write(" ********")
    Console.SetCursorPosition(hPos, vPos + 6)
    Console.Write("  ******")
End Sub
Nathan Tuggy
  • 2,237
  • 27
  • 30
  • 38
Calvin P.
  • 1,116
  • 2
  • 8
  • 13

3 Answers3

1

I'd be looking at this type of construct:

If 1 <= i <= 3 Then

I'm pretty certain that would evaluate 1 <= i and then compare the result of that with 3. Since the result of a boolean expression is either -1 for true or 0 for false, that expression would always evaluate as true.

You should instead opt for something like:

If 1 <= i And i <= 3 Then ' could also use AndAlso '

(and ditto for the other similar lines).

paxdiablo
  • 854,327
  • 234
  • 1,573
  • 1,953
  • Thank you! Can't believe I didn't think of that haha... I spent more time in java than vb and I thought it always worked in java. Anyway I adapted this to i<4, i<7, else – Calvin P. Apr 15 '15 at 02:16
1

The line of code below (and other lines like it) does not do what you think:

If 4 <= i <= 6 Then

What actually happens here is that first just the 4 <= i expression is evaluated. Then the boolean result from that expression is used for the <= 6 comparison. To make this comparison, the boolean must first be converted to an integer. The boolean conversion back to integer results in either a 0 (false) or -1 (true) (reference). Both of those are less than 6, and so this full line of code will always result in True.

You need to write it like this, instead:

If 4 <= i AndAlso i <= 6 Then

BTW, are you sure Option Strict is turned on? Because it should be, and if it were I'd expect a compiler error when you try to compare a boolean to an integer. Don't write code without Option Strict, or at least Option Infer. They help greatly to avoid mistakes like this.

Community
  • 1
  • 1
Joel Coehoorn
  • 399,467
  • 113
  • 570
  • 794
1

While the answers given so far are correct - you should be using AndAlso - there is another way of looking at the issue.

Your entire For i As Integer = 1 To 9 loop can be simply replaced by this:

    For Each i In x
        drawX(hor + hSpace * ((i - 1) Mod 3), ver + vSpace * ((i - 1) \ 3))
    Next
    For Each i In o
        drawO(hor + hSpace * ((i - 1) Mod 3), ver + vSpace * ((i - 1) \ 3))
    Next

This is more directly computing the position of the letter.

I would even go further and rewrite your code like this:

Option Strict On

Module Module1

    Sub Main()
        Dim x = New List(Of Integer)() From {2, 4, 7}
        Dim o = New List(Of Integer)() From {1, 5, 9}
        While True
            board(x, o)
            pause()
        End While
    End Sub

    Sub pause()
        Console.WriteLine("Press enter to continue...")
        Console.ReadLine()
    End Sub

    Sub board(x As List(Of Integer), o As List(Of Integer))
        Console.ForegroundColor = ConsoleColor.DarkGray

        Dim vSpace As Integer = 11
        Dim hSpace As Integer = 20
        Dim ver As Integer = 1
        Dim hor As Integer = 4

        For i = 1 To 2
            For j = 0 To 3 * (vSpace - 1)
                Console.SetCursorPosition(i * hSpace - 2, j)
                Console.Write("**")
            Next
        Next

        For j = 1 To 2
            Console.SetCursorPosition(0, j * vSpace - 2)
            Console.Write("".PadRight(3 * hSpace - 2, "*"c))
            Console.SetCursorPosition(0, j * vSpace - 2 + 1)
            Console.Write("".PadRight(3 * hSpace - 2, "*"c))
        Next

        For Each i In x
            drawX(hor + hSpace * ((i - 1) Mod 3), ver + vSpace * ((i - 1) \ 3))
        Next
        For Each i In o
            drawO(hor + hSpace * ((i - 1) Mod 3), ver + vSpace * ((i - 1) \ 3))
        Next

    End Sub

    Sub drawX(hPos As Integer, vPos As Integer)
        drawLetter(hPos, vPos, ConsoleColor.DarkYellow,
        {
            "**      **",
            " **    **",
            "  **  **",
            "   ****",
            "  **  **",
            " **    **",
            "**      **"
        })
    End Sub

    Sub drawO(hPos As Integer, vPos As Integer)
        drawLetter(hPos, vPos, ConsoleColor.DarkCyan,
        {
            "  ******",
            " ********",
            "**      **",
            "**      **",
            "**      **",
            " ********",
            "  ******"
        })
    End Sub

    Sub drawLetter(hPos As Integer, vPos As Integer, color As ConsoleColor, letter As String())
        Console.ForegroundColor = color
        For i = 0 To 6
            Console.SetCursorPosition(hPos, vPos + i)
            Console.Write(letter(i))
        Next
    End Sub

End Module
Enigmativity
  • 113,464
  • 11
  • 89
  • 172
  • While I likely won't be revising my code to this extent for this particular program, I like your suggestions and will keep them in mind for future developments. =) I also have to say thank you for the time and effort you put into this response. – Calvin P. Apr 16 '15 at 02:17
  • @user3582362 - No trouble. It was a fun exercise in refactoring. I hope that you might be able to use some of the techniques in the future. – Enigmativity Apr 16 '15 at 02:29