PDA

View Full Version : Optimization with for each



lw22
06-29-2009, 02:34 PM
I am not sure if this is the correct forum or even site but I'm looking for a little help with an Excel VBA macro. I have been reading up on VBA coding and found that (for the most part) you should avoid using loops. Also, you should use a for each loop rather than a regular for loop when possible. This is where my problem lies.

I have a program that pulls data from a database with a custom Excel function. Since it takes awhile for the function to run after the data populates I wrote a macro to copy the data and paste it to a sheet where it can be logged. First in Excel I renamed the automatic input sheet to sInput and the data logging sheet to sFill. When the program runs it checks the date the data was gathered and returns the corresponding row in sFill. Then it loops through the names in sInput to the names in sFill. If the names are the same it copies the data.

To loop through the names I am using for loops. Is there a way to range the cells the names are in, do the comparison, and return the row where the names are equal? The code is below for your reference. If you need the actual file I can provide that as well.

Option Explicit
Sub MoveToFill()
''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
'Moves data from the automatic input sheet to the data logging sheet
''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''

Dim i As Long, rInput As String, rFill As String, rDates As String, j As Long, lngDate As Long
Dim strInput As String, strFill As String, lngInput As Long, lngFill As Long, lngGPM As Long
Dim m As Long

'Set ranges for the tank comparison - not set up yet
' rInput = "D10:D14"
' rFill = "C13:S13"
' rDates = "A6:A9"

'Find date the data is for
For m = 6 To 9
If sFill.Cells(m, 1) = sInput.Cells(4, 5) Then
lngDate = m
Exit For
Else
MsgBox "Warning - Date entered is invalid"
Exit Sub
End If
Next m


'Loop through to put data at matching Fill data
For i = 10 To 14
'Only copies data if an x is placed in the adjacent cell
If sInput.Cells(i, 3) = "x" Then
'Loop through Fill names
For j = 3 To 19 Step 4
'If names are the same it copies the auto data
If sInput.Cells(i, 4) = sFill.Cells(13, j) Then
sFill.Cells(lngDate, j) = sInput.Cells(i, 5)
sFill.Cells(lngDate, j + 1) = sInput.Cells(i, 6)

'Adds Name3's rate since it is calculated seperate
If sInput.Cells(i, 4) = "Name3" Then
sFill.Cells(lngDate, j + 3) = sInput.Cells(i, 7)
End If

Exit For
End If
Next j
End If
Next i

sFill.Activate
End Sub

I do have another question... This code is placed in ThisWorkbook. On the sInput sheet I have a button that I use to call the ThisWorkbook.MoveToFill() sub. I was playing around with changing the name property of all of the sheets and I renamed the name of ThisWorkbook to wLog. When I try to run wLog.MoveToFill() the program doesn't work. Any thoughts on why this is happening?

rbrhodes
06-29-2009, 03:01 PM
Hi lw22,

Loops aren't evil per se, just slow(er). A For Each loop is reputedly faster than a regular For Loop stucture. A .Find command might be the way to go as you're looking for dupe names here. Perhaps post the file (or an example of it.

As for the second question it appears the buton can't find the macro. I'd check the names a little more closely.

Cheers,

dr

lw22
06-29-2009, 04:06 PM
Well now it is not seeming to match up now for some reason. The one at work I have working fine and I tried to recreate this one. It is not matching the ranges I think since the cells are merged but I didn't play around with it yet. Below is the file.

*Fixed the problem. The files works like I planned but is there a way to do with with for each?

rbrhodes
06-29-2009, 08:24 PM
[edit: Not lost just no attachment.]

rbrhodes
06-29-2009, 08:27 PM
Looks like I lost my post...

Anyways, here it is commented and all. I added a 'Clear button', changed all to For/Each and moved the subs to Modules. I can see no reason why they would want to be in the ThisWorlbook Module and they repeatedly *cacked* when called from there.

Note: Both Subs have a "Name3" in them. This will have to be changed unless "Name3" is the actual name.

Try post again!

lw22
06-30-2009, 02:50 PM
Thanks that is informative. I have a couple of questions though since I am not familiar with VBA.

What is the difference from putting it in a Module or in ThisWorkbook? I thought the only difference was the code in ThisWorksbook was private to only code in that workbook. Like you cannot pass variables in individual worksheets between worksheets.

What is the benefit of using the "Set" method of setting cells equal to variables rather than just using the variable = whatever?

Lastly, what all is the Find.(What:) useful for. I have never seen something like that before. What all is this useful for?

Thanks for the response

rbrhodes
06-30-2009, 06:59 PM
Hi lw22,

The only reason I moved it to a module was that it was routinely *cack*ing when called by the button. As far as I know the ThisWorkbook module is for Workbook_Open, and global sheet events. The Sheet modules are for sheet specific code and everything else goes into a module. The Subs can be marked Private if desired.

Hmmn, I guess I don't know!

A variable is just that, a variable. Equal to a certain type (Variable, String, Long, etc.) Using Set creates an object ie a range object that Excel can reference and work with.

MyRange = Range("A1") means that MyRange contains the value) of cell A1.

Set Myrange = Range("A1") means that MyRange IS the range A1.

So:

Set Myrange = Range("A1:A200") allows you to do something with the range variable, as in:

for each cel in MyRange
...do something to cell....
next cel

Look up 'Find' in VBA help. Choose Find Method from the available list. (It's the same command as on the Edit/Find menu.)

There is a very adaptable example there.

Or turn on macro recording the Edit/Find and Find next. Stop recording and view the macro.


Option Explicit
Sub Macro1()

Cells.Find(What:="aaa", After:=ActiveCell, LookIn:=xlFormulas, LookAt:= _
xlPart, SearchOrder:=xlByRows, SearchDirection:=xlNext, MatchCase:=False _
, SearchFormat:=False).Activate
Cells.FindNext(After:=ActiveCell).Activate
End Sub



I simply dumped all the options except 'Find what'