Welcome to OGeek Q&A Community for programmer and developer-Open, Learning and Share
Welcome To Ask or Share your Answers For Others

Categories

0 votes
1.2k views
in Technique[技术] by (71.8m points)

vba - Make it faster?

I am currently with a bit of a dilemma. After months working on a Macro/vba project the thing's gotten massive (about 4K lines of code) and because it has to loop through many lists several times, it can sometimes take up to half an hour before it finishes, sometimes even stopping on its own without any apparent reason (or error message).

I found out that even after turning off the screen update, though faster, it still would save me about 5/10 minutes of processing.

So my question is this:

How much of a difference in speed would there be if the program didn't use "Variable = Cells(1, x)" and instead went for "Variable = Worksheets("Sheet1").Cells(1,x)"?

Because it switches between tabs just twice throughout the whole thing, would it be worth it to start rewriting it so that it doesn't?

Please consider all my VBA knowledge has been self taught through trial and error, so use small words if possible.

EDIT:

I get 3 sheets with a bunch of data (I don't create these and neither can I change what/how the information is shown).

Sheet A has a list of my company's clients along with who's in charge of each account.

Sheet B consists of data detailing things those clients have bought/sold to my company (with information such as costs, earning, size, product, etc) over the past 2 years.

Sheet C contains what new products we've managed to make a new 'promise' for (So if one of our workers manages to get company 1 to claim they'll buy from us product X from now on, that 'promise' will appear listed here).

What I've been asked to do (and my project) is firstly fill out the details of each client on Sheet A with the information from Sheet B (So for client A we'll now see that they bought X€ in 2014), then it will create a new sheet showing all the 'promises' from sheet C that have been 'fulfilled' in Sheet B (The reason we detail these is because the offered prices (per unit) will be lower to clients that've 'promised' to buy certain amounts. The problem is that many clients take advantage of this and promise more than what they actually buy, so we had a need to make this to see who was keeping their word or not).

At this point, I create a "Main.xlsm" and dump the information there, and from there I've got to make excels for each of our employees detailing only the information about their clients (with about 56 employees you can figure why I desperately went to make this into a program rather than do it by hand every time they ask for this 'report' (which is twice a month at least)).

The main problem I'm having in not using the dreaded ".select" option is that I've been asked to give specific format to certain parts of my report, that're highly dependent on multiple variables. This along with a lot of checks to make sure the data isn't faulty somehow makes the 'bulk' in lines of code in my program as they're a lot of things like "If last year they had X% more deliveries but they paid Y% less than the one before, then you've got to blablablabla, and if it was Z% less you have to..." or "If the client's made purchases last year but not this one, you've got to copy the whole line from Sheet A (format included) and put it on a new sheet you'll name 'No Purchases'".

I had started using stuff like:

With Range(Cells(row, detUnit), Cells(BottomRow, detUnit + 2)).Borders(xlEdgeTop)
       .LineStyle = xlContinuous
      .ColorIndex = 0
    .TintAndShade = 0
          .Weight = xlThin
End With

For the formatting, but I've also got a lot of:

Columns(prtFilesFORWINAnoMes2).Select
Selection.Replace What:=" ", Replacement:="", LookAt:=xlPart, SearchOrder:=xlByRows, MatchCase:=False, SearchFormat:=False, ReplaceFormat:=False
Columns(prtAfrKgAnoMes1).Select
Selection.Replace What:=" ", Replacement:="", LookAt:=xlPart, SearchOrder:=xlByRows, MatchCase:=False, SearchFormat:=False, ReplaceFormat:=False

Where I've got to 'delete' potential mistakes whoever it is that inputted the data for Sheets A,B,C could've made (and there are a LOT).

To save a bunch of time, I also got to reorganize the various sheets in various ways, so I've also got

Rows("1:1").Select
Selection.AutoFilter
If ActiveSheet.AutoFilterMode = False Then Selection.AutoFilter     

ActiveWorkbook.Worksheets(ActiveSheet.Name).AutoFilter.Sort.SortFields.Clear        
ActiveWorkbook.Worksheets(ActiveSheet.Name).AutoFilter.Sort.SortFields.Add Key:=Cells(1, columnaDestiny), SortOn:=xlSortOnValues, Order:=xlAscending, _
    DataOption:=xlSortTextAsNumbers
With ActiveWorkbook.Worksheets(ActiveSheet.Name).AutoFilter.Sort
         .Header = xlYes
      .MatchCase = False
    .Orientation = xlTopToBottom
     .SortMethod = xlPinYin
    .Apply
End With
ActiveWorkbook.Worksheets(ActiveSheet.Name).AutoFilter.Sort.SortFields.Add Key:=Cells(1, columnaOrigin), SortOn:=xlSortOnValues, Order:=xlAscending, _
    DataOption:=xlSortTextAsNumbers
With ActiveWorkbook.Worksheets(ActiveSheet.Name).AutoFilter.Sort
         .Header = xlYes
      .MatchCase = False
    .Orientation = xlTopToBottom
     .SortMethod = xlPinYin
    .Apply
End With

Practically at the beginning of every Sub.

As an example, this is the main piece of code from the "See if promises in sheet C are fulfilled in Sheet B"

Do While row <= rowLength
    first = CoutryReference     'I've got to know which is the main country
    Select Case Cells(row, detOrigin)
    Case Is = CoutryReference
           dir = "EXPORT"      'Determine if it's Export / Import / Xtrade
        second = Cells(row, detDestiny)
    Case Else
        If Cells(row, detDestiny) = CoutryReference Then
               dir = "IMPORT"
            second = Cells(row, detOrigin)
        Else
               dir = "XTRADE"
             first = Cells(row, detOrigin)
            second = Cells(row, detDestiny)
        End If
    End Select
    If SearchInForwin(dir, first, second, Cells(row, detClient), Cells(row, detProduct)) = True Then Call FoundLine(row, dir)
    'SearchInForwin will loop through the (already organized) list in Sheet B and if it finds a match
    ' it will copy that line to Sheet "Fulfilled" and return "TRUE"
    ' FoundLine will then copy the line we're currently reading the information from and paste it into "Fulfilled" as well
    row = row + 1

Loop

And This is SearchInForwin:

Function SearchInForwin(direction As String, onecountry As String, othercountry As String, company As String, mode As String) As Boolean

Sheets("SHEET B").Select
Dim foUnd As Boolean, lookingRow As Long

lookingRow = lastHiddenWon      'Since it's alphabetical by Company, with
     foUnd = False              ' this we can jump to the last one found and start from there

Do While lookingRow <= Cells(Rows.Count, forwOrigen).End(xlUp).row
    If Cells(lookingRow, forwEmpresa) = company Then
        foUnd = True                                'First Loop it to quickly determine if there's a simple match
        If Cells(lookingRow, forwDireccion) = direction Then GoTo SecondBuc
    End If
    If (Cells(lookingRow, forwEmpresa) <> company And foUnd = True) Or Cells(lookingRow, forwAno) < yearAno Then
        foUnd = False 'This is because we should only take into account purchase data from the latest year (and it's pre-organized so the most recent data is on the top of the list)
        GoTo FIn
    End If
    lookingRow = lookingRow + 1
Loop

SecondBuc:
foUnd = False
Do While Cells(lookingRow, forwEmpresa) = company And Cells(lookingRow, forwDireccion) = direction And Cells(lookingRow, forwAno) = yearAno
        'The conditions are the only thing that keeps this second loop extremely short
    If Cells(lookingRow, forwAno) = yearAno And Cells(lookingRow, forwDestino) = othercountry And _
        Cells(lookingRow, forwOrigen) = onecountry And InStr(1, Cells(lookingRow, forwTIPO), mode) > 0 Then
            Call CopyToHidden(lookingRow, mode) 'Copies the line
                    foUnd = True
            lastHiddenWon = lookingRow + 1
    End If
    lookingRow = lookingRow + 1
Loop

FIn:            
SearchInForwin = foUnd
End Function

I could upload a .bas of my modules, but all the comments/variables are in Spanish because I'm supposed to make it 'understandable' for potential coworkers that might have to take a look at it (Meaning they want to be able to fire me and have someone else continue my work if they feel like it)

See Question&Answers more detail:os

与恶龙缠斗过久,自身亦成为恶龙;凝视深渊过久,深渊将回以凝视…
Welcome To Ask or Share your Answers For Others

1 Reply

0 votes
by (71.8m points)

If the question is, will the speed improve to switch from something Like:

For i = 1 to 4000
If Cell(i,2) = X then 
...do something
EndIf

to

Set Ranges = Cell(i,2),Cell(4000,2)
Loop through Variable

Then the answer is it will be WAY WAY faster. I had a similar situation where there was about 10-15 sheets ranging from 1000 rows to 900K rows. Check cell by cell, took 30-60 min, when i put it in variables it was under 5 min. Variables are ALWAYS faster than reading from a sheet, at least in my experience. I suppose if your reading 1 or 2 times it probably isn't noticeable. p.s. not real code above.


与恶龙缠斗过久,自身亦成为恶龙;凝视深渊过久,深渊将回以凝视…
OGeek|极客中国-欢迎来到极客的世界,一个免费开放的程序员编程交流平台!开放,进步,分享!让技术改变生活,让极客改变未来! Welcome to OGeek Q&A Community for programmer and developer-Open, Learning and Share
Click Here to Ask a Question

...