Click to See Complete Forum and Search --> : is this too many comments?


RickyD
March 11th, 2008, 05:18 AM
My instructor wants us to make sure we have a good use of comments in our mid term project

am I going a bit over board with my comments?

This is my completed and working code

in real world programming do you guys use a lot of comments?


' Created by Rick Dobbelmann on 3-10-08 for VB.NET Part 1 Midtem Project
' The purpose of this application is for a user to try and guess a random number
' created by the application. The player receives a rating depending on how many
' guesses the player needs to guess the correct number

Public Class xMainForm
' declare variables
Dim name As String
Dim getName As String
Dim randomNumber As Integer
Dim guessedNumber As Integer = 0
Dim guessComplier As Integer
Dim gameCounter As Integer = 0
Dim guessCounter As Integer = 0
Const wayTooLow As String = " - Way too Low, try again"
Const wayTooHigh As String = " - Way too high, try again"
Const tooLow As String = " - Too low, try again"
Const tooHigh As String = " - Too High, try again"
Dim rating As String
Dim randomGenerator As New Random
Dim isConverted As Boolean


Private Sub xMainForm_Load(ByVal sender As System.Object, ByVal e As System.EventArgs) Handles MyBase.Load

' When the form loads an input box will show asking the user for their name
' if the user doesnt enter a name the default player name will be "Player"
getName = InputBox("Please enter your name:", "Name Entry!", "Player")
name = getName.ToString

' disable play again button until the game is completed
Me.xPlayAgainBtn.Enabled = False

' display a welcome lable to the user and also display their name.
Me.xWelcomeLabel.Text = Me.xWelcomeLabel.Text & " " & name & ","

' When the form loads generate a random number between 1 and 100
Call GenerateRandomNumber()

' set the focus to the guessed number text box.
Call SetFocus()

End Sub

Private Sub xExitBtn_Click(ByVal sender As System.Object, ByVal e As System.EventArgs) Handles xExitBtn.Click

' when the player exits the game display the players name, the number of games played,
' and the average number of guesses per game

Dim guessAvg As Integer

' catch the exception if no games are played,
' handle it by assigning the value of 0 to the guess average variable
Try
guessAvg = guessComplier / gameCounter
Catch ex As Exception
guessAvg = 0
End Try

MessageBox.Show("Thanks for playing " & name & ControlChars.NewLine _
& "You played " & gameCounter & " games." & ControlChars.NewLine & "You averaged " _
& guessAvg & " guesses per game", "Your game stats!", MessageBoxButtons.OK, MessageBoxIcon.Information)
Me.Close()

End Sub

' sub method to generate a random number
Private Sub GenerateRandomNumber()
randomNumber = randomGenerator.Next(1, 101)
End Sub

Private Sub xGuessBtn_Click(ByVal sender As System.Object, ByVal e As System.EventArgs) Handles xGuessBtn.Click

' check and see if the number entered can be converted to an integer
' otherwise display an error message
isConverted = Integer.TryParse(Me.xNumberTxtBox.Text, guessedNumber)

' if number can be converted
If isConverted Then

' then if the guessed number is between 1 and 100
If guessedNumber > 0 And guessedNumber < 101 Then

' convert the number entered into a integer
guessedNumber = Convert.ToInt16(Me.xNumberTxtBox.Text)

' add 1 to the guess counter
guessCounter = guessCounter + 1

' if the user guesses the correct number
' display results in a message box & current game label
If guessedNumber = randomNumber Then

' declare variables
Dim youGuessed As String = "You guessed "
Dim times As String = " times. "
Dim cTitle As String = name & "'s " & "rating"
Dim tempLabel As String



' add the total guesses to the guess complier
guessComplier = guessComplier + guessCounter

' add one to the game counter
gameCounter = gameCounter + 1

' display the guess number counter and the word correct in the current game label
Me.xCurrentGameLabel.Text = "Guess: " & guessCounter & " - Correct" & ControlChars.NewLine _
& Me.xCurrentGameLabel.Text



' assign a rating message to the rating string variable
' determined by the guess counter
If guessCounter <= 3 Then
rating = "Great Work! You are a mathematical wizard"
ElseIf guessCounter > 3 And guessCounter <= 7 Then
rating = "Not too bad! You have some potential"
ElseIf guessCounter > 7 Then
rating = "What took you so long? Maybe you need some lessons."
End If


' display a message box containing the number of guesses
' and the players rating
MessageBox.Show(youGuessed & guessCounter & times & rating, _
cTitle, MessageBoxButtons.OK, MessageBoxIcon.Information)



' disable guess button until play agin button is pressed
Me.xGuessBtn.Enabled = False

' enable play agin button
Me.xPlayAgainBtn.Enabled = True


' use the tempLabel to store the current text in the all games label
' I use temp labels in the elseIf statements too
tempLabel = ControlChars.NewLine & ControlChars.NewLine & Me.xAllGamesLabel.Text

' add the completed games score to the all games label
' include the game number, the random number,
' the number of guesses and the players rating
' keep the most current guess at the top
Me.xAllGamesLabel.Text = "Game number: " & gameCounter & ControlChars.NewLine _
& "Random Number: " & randomNumber & ControlChars.NewLine & "Guesses: " & guessCounter _
& ControlChars.NewLine & "Rating: " & rating & tempLabel

Me.xPlayAgainBtn.Focus()



' if the random number is less then the guessed number
' and if the user is greater then 10 numbers off
' display a message telling the user they are way too high
' or if the user is less then 10 numbers from the correct number
' display a message telling them they are too high.
' keep the most current guess at the top
ElseIf randomNumber < guessedNumber Then
Dim tempLabel As String
Dim wayToo As Integer

tempLabel = ControlChars.NewLine & Me.xCurrentGameLabel.Text
wayToo = guessedNumber - randomNumber
If wayToo >= 10 Then
Me.xCurrentGameLabel.Text = "Guess: " & guessCounter & wayTooHigh & tempLabel
Call SetFocus()
ElseIf wayToo < 10 Then
Me.xCurrentGameLabel.Text = "Guess: " & guessCounter & tooHigh & tempLabel
Call SetFocus()
End If




' if the random number is greater then the guessed number
' and if the player is greater then 10 numbers off
' display a message telling the user they are way too low
' or if they the user is less then 10 numbers from the correct number
' display a message telling them they are too low.
' keep the most current guess at the top
ElseIf randomNumber > guessedNumber Then
Dim tempLabel As String
Dim wayToo As Integer

tempLabel = ControlChars.NewLine & Me.xCurrentGameLabel.Text
wayToo = randomNumber - guessedNumber

If wayToo >= 10 Then
Me.xCurrentGameLabel.Text = "Guess: " & guessCounter & wayTooLow & tempLabel
Call SetFocus()
ElseIf wayToo < 10 Then
Me.xCurrentGameLabel.Text = "Guess: " & guessCounter & tooLow & tempLabel
Call SetFocus()
End If
End If



Else
' if the number entered is 0 or the number entered is greater then 100 display an error message
MessageBox.Show("Please enter a number between 1 and 100", "Entry Error!", _
MessageBoxButtons.OK, MessageBoxIcon.Error)
Call SetFocus()

End If
Else
' if the numbered entered cannot be converted to a valid integer display an error message
MessageBox.Show("Please enter a number between 1 and 100", "Entry Error!", _
MessageBoxButtons.OK, MessageBoxIcon.Error)
Call SetFocus()
End If
End Sub


Private Sub xPlayAgainBtn_Click(ByVal sender As System.Object, ByVal e As System.EventArgs) Handles xPlayAgainBtn.Click
Call GenerateRandomNumber()

' enable the guess button
Me.xGuessBtn.Enabled = True

' disable the play agin button until the next game finishes
Me.xPlayAgainBtn.Enabled = False

' reset the guess counter
guessCounter = 0

' reset the current game label and set the focus to the text box
Me.xCurrentGameLabel.Text = ""
Call SetFocus()
End Sub


' only allow the numbers 0 - 9 and the back space button to be
' entered in the guess number text box
Private Sub xNumberTxtBox_KeyPress(ByVal sender As System.Object, ByVal e As System.Windows.Forms.KeyPressEventArgs) Handles xNumberTxtBox.KeyPress
If (e.KeyChar < "0" OrElse e.KeyChar > "9") _
AndAlso e.KeyChar <> ControlChars.Back Then
e.Handled = True
End If
End Sub


' simple method to set the focus and select all text in the guessed number text box
Private Sub SetFocus()
Me.xNumberTxtBox.Focus()
Me.xNumberTxtBox.SelectAll()
End Sub


' if the user doesnt want to see the current game results
' they can uncheck the current game check box
' and then the current game label will be hidden
Private Sub DisplayCurrentGame(ByVal sender As System.Object, ByVal e As System.EventArgs) Handles xCurrentResultsChkBox.CheckedChanged
If Me.xCurrentResultsChkBox.Checked = False Then
Me.xCurrentGameLabel.Visible = False
Else
Me.xCurrentGameLabel.Visible = True
Call SetFocus()
End If

End Sub


' if the user doesnt want to see the all of the games they have completed,
' the player can uncheck all games check box and then the
' all games completed label will be hidden
Private Sub DisplayAllGames(ByVal sender As System.Object, ByVal e As System.EventArgs) Handles xAllResultsChkBox.CheckedChanged
If Me.xAllResultsChkBox.Checked = False Then
Me.xAllGamesLabel.Visible = False
Else
Me.xAllGamesLabel.Visible = True
Call SetFocus()
End If
End Sub
End Class

satanorz
March 11th, 2008, 07:28 AM
Indeed, my code is green as Hulk :D

I personally think that your code is well commented, and yeah, it's better this way.. i've seen a lot of medium/big project's that no one can decypher because has 0 comments.

:wave:

HanneSThEGreaT
March 11th, 2008, 08:12 AM
Good Job Ricky!
If you were one of my students, I'd be happy.

I just have some thoughts:
Try to summarise the keypoints. Meaning, try not to explain every little thing in one paragraph, but, just, say for example, after a statement just give a quick indication of what that line's purpose is. Take this for example :

Private Sub DisplayAllGames(ByVal sender As System.Object, ByVal e As System.EventArgs) Handles xAllResultsChkBox.CheckedChanged
If Me.xAllResultsChkBox.Checked = False Then 'Don't want to see all completed games
Me.xAllGamesLabel.Visible = False 'Hide
Else 'They want to
Me.xAllGamesLabel.Visible = True 'Show games
Call SetFocus() 'Set the focus
End If
End Sub

It still gives the same impression, but I suppose it's just a matter of preference. IMHO, as I said, yours is quite good, and if I were your trainer, you would definitely get good marks :thumb:

Also, are you using VS 2005 ¿
Because you can add summary info for each sub. For example, if you were to enter 3 apostrophes, like :
'''
You'd get a space where you can add that type of info above each of your subs.

Good work!

JohnW@Wessex
March 11th, 2008, 09:07 AM
HanneSThEGreaT (http://member.php?u=56188)
Member With Special PowerLeaps tall buildings in a single bound?
Faster than a speeding bullet? :eek:

(I've never seen that tag before)

RickyD
Summary comments before a block of code is good, especially where thealgorithm is not obvious from the code. Try not to go overboard and comment the obvious though.

HanneSThEGreaT
March 11th, 2008, 09:53 AM
Leaps tall buildings in a single bound?
Faster than a speeding bullet? :eek:

(I've never seen that tag before)

LOL! In my wildest dreams! :D

No, seriosuly, when you reach a certain number of posts, you can customise your "Title". I chose that, well, because of too many things to mention :)

Good answer though! :thumb:

MikeVallotton
March 11th, 2008, 10:18 AM
Yes, real world, that is what my code typically looks like. I do the same as HannesTheGreat with abbreviating some of the comments, but I usually have 1 comment for every 2-3 lines of code.

Makes it much easier to pick up a year or two later when you don't remember what you were doing.

bflosabre91
March 11th, 2008, 12:42 PM
you never can have too many comments. i wish i would comment like that, but I am already in the habit of not commenting as much as i should. Keep it up and make a habit of good comments. If there really are too many comments, then you can easily get rid of them before your application goes into production so you are def good to go

dglienna
March 11th, 2008, 01:51 PM
It all depends on who will be looking at the code. If it's only me, then I'll comment sparingly. If someone else will see it, then I'll do the same thing, generally. '

Craig Gemmill
March 11th, 2008, 03:09 PM
Assuming you write and "refactor" your code as efficiently as possible (or, at least, pretty well) you can usually write a summary comment at the beginning of each method to explain what's going on inside. Commenting each line is often more confusing and time consuming for the writer and the viewer.

Obviously there will be cases where you do something completely weird (to work around a bug, etc), in which case you'll want to comment that line directly.

Today, take that block of code you have posted and comment it line-by-line. Then make a copy and comment it using summaries instead. Save them in a safe place. Set an appointment in your calendar to review them in 1-2 months.

See which one makes the most sense to you then, and adopt that method moving forward.

RickyD
March 11th, 2008, 10:48 PM
thanks guys for your comments, I feel better now that you guys approve and think I did well.

RickyD
March 11th, 2008, 10:50 PM
Good Job Ricky!


Also, are you using VS 2005 ¿

Good work!

opps forgot to post that I am using vb.net 2005