100% endorse David's point about using Option Explicit, no serious coder should not use it.
I disagree with David about the comments. To my mind as comments these are totally superfluous, they tell you nothing that is not obvious from the code. The only reason I can believe that they are there is to provide an instant visual cue to coding blocks, in which case column one is the best place for them in my view. If they are meant to be real comments, they fail and I would ditch them.
You can also reduce the code where the checkboxes are tested and make it more readable by changing
'Check Box Input
If chkWin.Value = True Then
Cells(erow, 4).Value = 1
Else
Cells(erow, 4).Value = 0
End If
If chkLoss.Value = True Then
Cells(erow, 5).Value = 1
Else
Cells(erow, 5).Value = 0
End If
If chkTie.Value = True Then
Cells(erow, 6).Value = 1
Else
Cells(erow, 6).Value = 0
End If
If chkHeld.Value = True Then
Cells(erow, 7).Value = 1
Else
Cells(erow, 7).Value = 0
End If
If chkDecision.Value = True Then
Cells(erow, 8).Value = 1
Else
Cells(erow, 8).Value = 0
End If
to
'Check Box Input
Cells(erow, 4).Value = -CLng(chkWin.Value)
Cells(erow, 5).Value = -CLng(chkLoss.Value)
Cells(erow, 6).Value = -CLng(chkTie.Value)
Cells(erow, 7).Value = -CLng(chkHeld.Value)
Cells(erow, 8).Value = -CLng(chkDecision.Value)
From: ExcelVBA@yahoogroups.com [mailto:ExcelVBA@yahoogroups.com]
Sent: 14 June 2014 01:37
To: ExcelVBA@yahoogroups.com
Subject: Re: [ExcelVBA] Would love feedback on first code
This is very simple code, so there's not much to comment on. However, I would recommend that you be more careful to be consistent in how you code, to make a more readable result. For example
- "Pitcher" is declared, but "erow" is not declared. You should declare all variables, and you should use "Option Explicit" to force this and allow Excel to check for errors.
- "Pitcher" has an initial capital, but "erow" does not. Be consistent in naming practices.
- "erow" is pretty meaningless. What does it mean? "EntryRow" I suspect. Name it that way.
- You (correctly) use the .Text attribute to grab the text of most of your text fields, but not for Pitcher. Use the attribute in every case.
- You have your comments in column 1 where they interrupt the flow of reading the code. Line your comments up with the code they refer to.
- You have used column numbers in the Cells calls, rather than column letters. Much more readable if you use the letters. E.g. Cells(EntryRow, "A") = txtDate.Text.
Other minor items
- Offset(1,0) is not needed in the calculation of EntryRow. Simply grab the row number and add 1 to it ... EntryRow = Sheet1.Cells(Rows.Count, 1).End(xlUp).Row + 1
- You've activated the worksheet whose name is in the Pitcher variable, but then you refer to Sheet1 to work out the EntryRow, and then you put the values into the active sheet. This is almost certainly incorrect.
Regards, Dave S
----- Original Message -----
Sent: Friday, June 13, 2014 10:34 PM
Subject: [ExcelVBA] Would love feedback on first code
If this is incorrect use of this forum, I apologize in advance. I have created a userform for populating data into a spreadsheet, and would love to hear feedback, suggestions, recommendations.
Private Sub cmdTransferStats_Click()
Dim Pitcher As String
Pitcher = txtPitcher
Worksheets(Pitcher).Activate
erow = Sheet1.Cells(Rows.Count, 1).End(xlUp).Offset(1, 0).Row
'Textbox Input
Cells(erow, 1) = txtDate.Text
Cells(erow, 2) = txtOpponent.Text
'Option Button Input
If optYes.Value = True Then
Cells(erow, 3).Value = 1
Else
Cells(erow, 3).Value = 0
End If
'Check Box Input
If chkWin.Value = True Then
Cells(erow, 4).Value = 1
Else
Cells(erow, 4).Value = 0
End If
If chkLoss.Value = True Then
Cells(erow, 5).Value = 1
Else
Cells(erow, 5).Value = 0
End If
If chkTie.Value = True Then
Cells(erow, 6).Value = 1
Else
Cells(erow, 6).Value = 0
End If
If chkHeld.Value = True Then
Cells(erow, 7).Value = 1
Else
Cells(erow, 7).Value = 0
End If
If chkDecision.Value = True Then
Cells(erow, 8).Value = 1
Else
Cells(erow, 8).Value = 0
End If
'More Textbox Input
Cells(erow, 9) = txtIP.Text
Cells(erow, 10) = txtStrikes.Text
Cells(erow, 11) = txtBalls.Text
Cells(erow, 12) = txtER.Text
Cells(erow, 13) = txtHits.Text
Cells(erow, 14) = txtBF.Text
Cells(erow, 15) = txtKs.Text
Cells(erow, 16) = txtWalks.Text
Cells(erow, 17) = txtHBP.Text
End Sub
No virus found in this message.
Checked by AVG - www.avg.com
Version: 10.0.1432 / Virus Database: 3955/7169 - Release Date: 06/13/14
Posted by: "Bob Phillips" <bob.phillips@dsl.pipex.com>
| Reply via web post | • | Reply to sender | • | Reply to group | • | Start a New Topic | • | Messages in this topic (3) |
Be sure to check out TechTrax Ezine for many, free Excel VBA articles! Go here: http://www.mousetrax.com/techtrax to enter the ezine, then search the ARCHIVES for EXCEL VBA.
----------------------------------
Visit our ExcelVBA group home page for more info and support files:
http://groups.yahoo.com/group/ExcelVBA
----------------------------------
More free tutorials and resources available at:
http://www.mousetrax.com
----------------------------------
Tidak ada komentar:
Posting Komentar