HiveBrain v1.2.0
Get Started
← Back to all entries
patternMinor

Simplifying manual data entry with a UserForm Excel 2010

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
entryexcelwithuserformmanualsimplifyingdata2010

Problem

I am an absolute beginner. This is the first thing I've ever coded in VBA. I don't know if I'm allowed to ask for just criticism in general without a question.

But just in case I can't, this file is going to get very large and I imagine I will have resource optimization problems - so what should I have done differently to compute these tasks efficiently?

The purpose of this code is to streamline some manual data entry I'm having to do at work. Basically, my department has a recruiting function and we post jobs on various websites. In order to develop some kind of metric, it is my job to record the performance data of jobs posted by all recruiters in department.

I've created this form in order to put some of this work back on the recruiters as part of their job posting workflow.

```
Public reqnum As String
Public jbtitle As String
Public jbloc As String
Public postdate As String
Public closedate As String
Public vws As Integer
Public apps As Integer
Public jobcategory As String
Public pidname As String

'If someone hits enter on the last textbox, it will submit the form
Private Sub ApplicantsBox_KeyDown(ByVal KeyCode As MSForms.ReturnInteger, ByVal Shift As Integer)
If KeyCode = 13 Then
Call UpdateButton_Click
End If
End Sub

'If someone hits enter while focused on the check box, they will change its value
Private Sub GoToCheckbox_KeyDown(ByVal KeyCode As MSForms.ReturnInteger, ByVal Shift As Integer)
If KeyCode = 13 Then
If GoToCheckbox.Value = True Then
GoToCheckbox.Value = False
Else
GoToCheckbox.Value = True
End If
End If
End Sub

Private Sub UpdateButton_Click()
'Establish variables with textbox values
reqnum = ReqNumBox.Value
jbtitle = JobTitleBox.Value
jbloc = JobLocationBox.Value
postdate = DatePostedBox.Value
closedate = DateClosedBox.Value
jobcategory = CategoryComboBox.Value
pidname = PIDComboBox.Value

'Check to make sure data in views and appl

Solution

Very fine effort and I see very little to change for efficiency reasons.

If it does start to slow down during your updates, you could turn off Application.ScreenUpdating and turn it back on after the UserForm has initialized.

But I don't see that as an issue now.

Some small suggestions below - I don't think they will do much for speed - but address readibility, maintenance and efficiency.


Improve readability by using predefined and system constants

EDIT - use constant at top for Carriage Return

Public Const CR As Integer = 13

Then Replace all checks for

KeyCode = 13
with
KeyCode = CR


Simplify

Change:

If GoToCheckbox.Value = True Then
GoToCheckbox.Value = False
Else
GoToCheckbox.Value = True
End If


To (just make sure you have default value = 0)

GoToCheckbox.Value = Not GoToCheckbox.Value


Practice common or even standard coding practice

  • Move your Dims to top of function/sub code




Simplify logical comparisons

-
no need to compare to true - or even to use a variable if you have a well-named function that indicates it returns true/false

-
continue to reference Sheets using With

Example:

Change

Sub FormatRows(rowtoformat As Long)
    'Extend Formulas
    With Sheets(1)
        .Range("F3:F" & rowtoformat).FillDown
        .Range("I3:I" & rowtoformat).FillDown
        .Range("J3:J" & rowtoformat).FillDown
    End With

    'Format Rows
    Dim even As Boolean
    even = IsEven(rowtoformat)
    If even = True Then
        Sheets(1).Rows(4).Copy
        Sheets(1).Rows(rowtoformat).PasteSpecial Paste:=xlFormats
    Else
        Sheets(1).Rows(5).Copy
        Sheets(1).Rows(rowtoformat).PasteSpecial Paste:=xlFormats
    End If
End Sub


To

Sub FormatRows(rowtoformat As Long)

    'Extend Formulas
    With Sheets(1)
        .Range("F3:F" & rowtoformat).FillDown
        .Range("I3:I" & rowtoformat).FillDown
        .Range("J3:J" & rowtoformat).FillDown

        'Format Rows    
        If IsEven(rowtoformat) Then
            .Rows(4).Copy
        Else
            .Rows(5).Copy
        End If
        .Rows(rowtoformat).PasteSpecial Paste:=xlFormats
    End With

End Sub


Hope that helps

Code Snippets

Sub FormatRows(rowtoformat As Long)
    'Extend Formulas
    With Sheets(1)
        .Range("F3:F" & rowtoformat).FillDown
        .Range("I3:I" & rowtoformat).FillDown
        .Range("J3:J" & rowtoformat).FillDown
    End With

    'Format Rows
    Dim even As Boolean
    even = IsEven(rowtoformat)
    If even = True Then
        Sheets(1).Rows(4).Copy
        Sheets(1).Rows(rowtoformat).PasteSpecial Paste:=xlFormats
    Else
        Sheets(1).Rows(5).Copy
        Sheets(1).Rows(rowtoformat).PasteSpecial Paste:=xlFormats
    End If
End Sub
Sub FormatRows(rowtoformat As Long)

    'Extend Formulas
    With Sheets(1)
        .Range("F3:F" & rowtoformat).FillDown
        .Range("I3:I" & rowtoformat).FillDown
        .Range("J3:J" & rowtoformat).FillDown

        'Format Rows    
        If IsEven(rowtoformat) Then
            .Rows(4).Copy
        Else
            .Rows(5).Copy
        End If
        .Rows(rowtoformat).PasteSpecial Paste:=xlFormats
    End With

End Sub

Context

StackExchange Code Review Q#134593, answer score: 2

Revisions (0)

No revisions yet.