PDA

View Full Version : Learning VBA



chaeling
12-14-2018, 03:01 PM
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.

Dave
12-16-2018, 11:23 AM
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!