patternMinor
Writing to many cells from many controls' respective value
Viewed 0 times
controlswritingrespectivevaluecellsmanyfrom
Problem
I have this program on Excel VBA, that implements a userform in order to simplify input and to create control charts from the data input analyze the data and send emails if certain conditions are met.
My problem is that everything works properly but after running line by line I noticed that when filling up the corresponding cells in excel with the user input on the userform the macro takes long to execute each line.
I am wondering if any of you have a different method of doing this that would reduce the time it takes to run through all these lines.
```
Function completeData(mo As String)
Dim row As Integer
row = moRow(mo)
DataHistory
'Tag Information
Worksheets("Data").Cells(row, 2).value = ComboBox1.value 'set UPC
Worksheets("Data").Cells(row, 4).value = TextBox2.value 'set DateCured
Worksheets("Data").Cells(row, 5).value = TextBox48.value 'set DateTested
Worksheets("Data").Cells(row, 41).value = ComboBox2.value 'set Grinder
Worksheets("Data").Cells(row, 42).value = ComboBox3.value 'set Metal
Worksheets("Data").Cells(row, 43).value = ComboBox4.value 'set Press
Worksheets("Data").Cells(row, 44).value = ComboBox5.value 'set Clock
Worksheets("Data").Cells(row, 45).value = ComboBox6.value 'set Oven
'weight 0min Wheels
Worksheets("Data").Cells(row, 15).value = TextBox3.value
Worksheets("Data").Cells(row + 1, 15).value = TextBox4.value
Worksheets("Data").Cells(row + 2, 15).value = TextBox5.value
'Before Thikness
''''FirstWheel
Worksheets("Data").Cells(row, 6).value = TextBox6.value
Worksheets("Data").Cells(row, 7).value = TextBox7.value
Worksheets("Data").Cells(row, 8).value = TextBox8.value
Worksheets("Data").Cells(row, 9).value = TextBox9.value
''''SecondWheel
Worksheets("Data").Cells(row + 1, 6).value = TextBox10.value
Worksheets("Data").Cells(row + 1, 7).value = TextBox11.value
Worksheets("Data").Cells(row + 1, 8).value = TextBox12.value
Worksheets("Data").Cells(row + 1, 9).value = TextBox13.value
''''ThirdWheel
Worksheets("Data").Cells(row + 2, 6).value = Tex
My problem is that everything works properly but after running line by line I noticed that when filling up the corresponding cells in excel with the user input on the userform the macro takes long to execute each line.
I am wondering if any of you have a different method of doing this that would reduce the time it takes to run through all these lines.
```
Function completeData(mo As String)
Dim row As Integer
row = moRow(mo)
DataHistory
'Tag Information
Worksheets("Data").Cells(row, 2).value = ComboBox1.value 'set UPC
Worksheets("Data").Cells(row, 4).value = TextBox2.value 'set DateCured
Worksheets("Data").Cells(row, 5).value = TextBox48.value 'set DateTested
Worksheets("Data").Cells(row, 41).value = ComboBox2.value 'set Grinder
Worksheets("Data").Cells(row, 42).value = ComboBox3.value 'set Metal
Worksheets("Data").Cells(row, 43).value = ComboBox4.value 'set Press
Worksheets("Data").Cells(row, 44).value = ComboBox5.value 'set Clock
Worksheets("Data").Cells(row, 45).value = ComboBox6.value 'set Oven
'weight 0min Wheels
Worksheets("Data").Cells(row, 15).value = TextBox3.value
Worksheets("Data").Cells(row + 1, 15).value = TextBox4.value
Worksheets("Data").Cells(row + 2, 15).value = TextBox5.value
'Before Thikness
''''FirstWheel
Worksheets("Data").Cells(row, 6).value = TextBox6.value
Worksheets("Data").Cells(row, 7).value = TextBox7.value
Worksheets("Data").Cells(row, 8).value = TextBox8.value
Worksheets("Data").Cells(row, 9).value = TextBox9.value
''''SecondWheel
Worksheets("Data").Cells(row + 1, 6).value = TextBox10.value
Worksheets("Data").Cells(row + 1, 7).value = TextBox11.value
Worksheets("Data").Cells(row + 1, 8).value = TextBox12.value
Worksheets("Data").Cells(row + 1, 9).value = TextBox13.value
''''ThirdWheel
Worksheets("Data").Cells(row + 2, 6).value = Tex
Solution
The first thing to do is to switch off calculation, sheet events and screen updating before you write anything to a worksheet, and restore it when you're done: that way you won't have Excel try to re-calculate the entire workbook whenever you write to a cell.
Next, name things, starting with the
Then you can do this:
In the form's designer (F7), give each control a meaningful name. For example
The problem with this is that you already have tons of code referring to these controls, and renaming
You'll need tools to do this cleanly. Fortunately Rubberduck can help with this refactoring:
You could right-click on a control, select Rubberduck > Rename (or right-click a
Rubberduck renames the associated event handler(s) and in-code identifier references for you - so there's no excuse for having meaningless identifiers anywhere anymore.
DISCLAIMER I'm heavily involved with the Rubberduck project.
Speaking of naming, it's not clear at all what
Procedures that don't return a value should be
Also, module members in VBA are implicitly
I'd strongly recommend reading up on VBA UserForm best practices, and restructure your code so that the form doesn't "run the show": the form should be there only to provide the rest of the code with user input, not to actually perform the worksheet manipulations - a form's code-behind should only be concerned about the form and its controls; a form that knows about a worksheet, plainly knows (and does) too many things.
I'd also suggest you define an enum to name each column of your "Data" sheet, so instead of this:
You would have that:
All that's needed is a simple declaration:
That makes it much easier to read the rest of the code, and seeing
Next, name things, starting with the
"Data" sheet. Select the worksheet in the Project Explorer (Ctrl+R), then bring up the Properties toolwindow (F4) and type DataSheet under the sheet's (name) property.Then you can do this:
With DataSheet
.Cells(...).Value = ...
.Cells(...).Value = ...
.Cells(...).Value = ...
.Cells(...).Value = ...
.Cells(...).Value = ...
.Cells(...).Value = ...
.Cells(...).Value = ...
...
End WithIn the form's designer (F7), give each control a meaningful name. For example
ComboBox1 contains a UPC code, so it should be named SelectedUPC.The problem with this is that you already have tons of code referring to these controls, and renaming
ComboBox1 to SelectUPC would break your code, because ComboBox1 would be left referring to nothing.You'll need tools to do this cleanly. Fortunately Rubberduck can help with this refactoring:
You could right-click on a control, select Rubberduck > Rename (or right-click a
ComboBox1 identifier reference in the code, select Rubberduck > Refactor > Rename), then specify SelectedUPC as the new name, and then...Rubberduck renames the associated event handler(s) and in-code identifier references for you - so there's no excuse for having meaningless identifiers anywhere anymore.
DISCLAIMER I'm heavily involved with the Rubberduck project.
Speaking of naming, it's not clear at all what
moRow means to be doing, nor who is calling that function, or even why it needs to be a Function.Procedures that don't return a value should be
Sub procedures; a Sub procedure executes a sequence of operations; a Function procedure computes a value and returns it to the caller.Also, module members in VBA are implicitly
Public, which means completeData (which should be CompleteData to be PascalCase like all other VBA procedures) is a public member of that UserForm.I'd strongly recommend reading up on VBA UserForm best practices, and restructure your code so that the form doesn't "run the show": the form should be there only to provide the rest of the code with user input, not to actually perform the worksheet manipulations - a form's code-behind should only be concerned about the form and its controls; a form that knows about a worksheet, plainly knows (and does) too many things.
I'd also suggest you define an enum to name each column of your "Data" sheet, so instead of this:
With DataSheet
.Cells(row, 2).Value = SelectedUPC.ValueYou would have that:
With DataSheet
.Cells(row, DataSheetColumns.UPC).Value = SelectedUPC.ValueAll that's needed is a simple declaration:
Private Enum DataSheetColumns
UPC = 2
DateCured = 4
DateTested = 5
...
End EnumThat makes it much easier to read the rest of the code, and seeing
DataSheetColumns.UPC being assigned to SelectedClock.Value looks much more wrong than if it's column 44 being assigned: making wrong code look wrong is even more important than writing code that "works".Code Snippets
With DataSheet
.Cells(...).Value = ...
.Cells(...).Value = ...
.Cells(...).Value = ...
.Cells(...).Value = ...
.Cells(...).Value = ...
.Cells(...).Value = ...
.Cells(...).Value = ...
...
End WithWith DataSheet
.Cells(row, 2).Value = SelectedUPC.ValueWith DataSheet
.Cells(row, DataSheetColumns.UPC).Value = SelectedUPC.ValuePrivate Enum DataSheetColumns
UPC = 2
DateCured = 4
DateTested = 5
...
End EnumContext
StackExchange Code Review Q#139373, answer score: 5
Revisions (0)
No revisions yet.