Consulting

Results 1 to 2 of 2

Thread: Learning VBA

  1. #1
    VBAX Newbie
    Joined
    Dec 2018
    Posts
    1
    Location

    Learning VBA

    If anyone has time to look at the attached file - I'd like to get some feedback and suggestions.

    I'm learning VBA, so I wrote a simple program like the game "Battleship!"

    I've got some basic logic working, and I'd like to get perspectives from any of you as to what I can improve or do differently.

    Please let me know what you think.
    Attached Files Attached Files

  2. #2
    VBAX Expert Dave's Avatar
    Joined
    Mar 2005
    Posts
    835
    Location
    It's Sunday and I'm bored. Looks like U have spent some time on this effort... a great way to learn VBA. Making a program into an interactive game is quite a challenge. U have nicely "chunked" up the code into smaller subs and added comments. A few things constructive... when U find repetitive code, usually there's a way to condense it. For example, U have multiple separate subs for setting ships...
    Sub Set_Cruiser()
    '********************************************
    ' Let's User place AirCraft Carrier
    '********************************************
    
    Dim ShipName As String
    Dim ExpSize As Integer
    ShipName = "Cruiser"
    ExpSize = 3
    Call CheckShipLenght(ShipName, ExpSize)
    If IsValid = True Then
        Call SetShip
    End If
    End Sub
    As a suggestion, U could change your CheckshipLength sub to a function...
    Public Function CheckShipLenght(ByRef ShipName As String, ByRef ExpSize As Integer) As Boolean
    CheckShipLenght = False
    Lenght = 0
    For Ci = MP_Col_Start To MP_Col_End
        For Ri = MP_Row_Start To MP_Row_End
            If Cells(Ri, Ci) = True Then
                Lenght = Lenght + 1
            End If
        Next Ri
    Next Ci
    If Lenght <> ExpSize Then
        MsgBox ("Remeber the " & ShipName & " must be " & ExpSize & " squares in size")
        'IsValid = False
    Else
    CheckShipLenght = True
    'IsValid = True
    End If
    End Function
    Then use the function like this...
    Sub Set_Destroyer()
    '********************************************
    ' Let's User place AirCraft Carrier
    '********************************************
    
    Dim ShipName As String
    Dim ExpSize As Integer
    ShipName = "Destroyer"
    ExpSize = 2
    If CheckShipLenght(ShipName, ExpSize) Then
    'If IsValid = True Then
        Call SetShip
    End If
    End Sub
    ...or better yet get rid of all those subs...
    If CheckShipLenght("Destroyer", 2) Then
    Call SetShip
    End If
    A few other hopefully constructive comments, the more modules the larger the file size. Always make sure Cells is preceded by a worksheet name or XL will use the active sheet which may or may not be what U want. Last but not least, always assume the end user is an idiot. A help button would be very useful as I really didn't understand how to play/operate the game. I knew the objective only by my previous game play... I can't comment on if the game actually works as I couldn't figure it out. HTH. Dave
    Welcome to this forum!

Posting Permissions

  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts
  •