0

I have this code from before, which I need to create 38 reports from the one big report. It worked before, but we added something, and now it doesn't work. I never understood Loop procedure well although I read other answers

So here is the section I believe is making the issue. It needs to go through 5602 lines and it takes minutes to even get to 50.

Do While v <= n
   If Cells(v, 2) <> "" And Cells(v, 2) <> "Call Center" And Cells(v, 2) <> drzava Then Rows(v).Delete Else v = v + 1
    Loop

And this is the whole thing:

Sub SaveALLCountries()

Dim drzava$, nov As Workbook, ime$, v%, n%, a  As Double

Application.ScreenUpdating = False

For i = 1 To 38
    ThisWorkbook.Activate
    Application.StatusBar = i
    ThisWorkbook.Sheets("Results by CC").Range("CB14") = i
    drzava = ThisWorkbook.Sheets("Results by CC").Range("CD12")
    Workbooks.Add
    Set nov = ActiveWorkbook
    ThisWorkbook.Sheets("Results by CC").Copy Before:=nov.Sheets(1)
    ActiveSheet.Shapes.Range(Array("List Box 2")).Delete
    Cells.Select
    Selection.Copy
    Selection.PasteSpecial Paste:=xlPasteValues, Operation:=xlNone, SkipBlanks _
    :=False, Transpose:=False
    Range("A1").Select
    ThisWorkbook.Sheets("2018 Q2 Open answers").Copy Before:=nov.Sheets(2)
    Application.DisplayAlerts = False
    nov.Sheets(3).Delete
    Sheets("2018 Q2 Open answers").Select
    ActiveSheet.Outline.ShowLevels RowLevels:=2
    n = Application.WorksheetFunction.CountA(Sheets("2018 Q2 Open answers").Columns(2)) + 10
    v = 1
    Do While v <= n
        If Cells(v, 2) <> "" And Cells(v, 2) <> "Call Center" And Cells(v, 2) <> drzava Then Rows(v).Delete Else v = v + 1
    Loop
    ActiveSheet.Outline.ShowLevels RowLevels:=1
    Range("A1").Select
    ActiveWorkbook.Names("CallCenterSelect").Delete
    Sheets("Results by CC").Select
    ime = ThisWorkbook.Path & "\" & Sheets("Results by CC").Range("CD14").Value & ".xlsx"
    nov.SaveAs ime, FileFormat:=xlOpenXMLWorkbook, CreateBackup:=False
    nov.Close

Next i

Application.ScreenUpdating = True
Application.StatusBar = False

End Sub

Micha Wiedenmann
  • 19,979
  • 21
  • 92
  • 137
  • Also setting calculation mode to manual might save some time, depending on the content of the cells. This will especially benefit if there are a lot of formula in the cells. – Luuklag Oct 16 '18 at 09:12
  • Also read on avoiding the use of select: https://stackoverflow.com/questions/10714251/how-to-avoid-using-select-in-excel-vba – Luuklag Oct 16 '18 at 09:14
  • It's not just that line - all that "select this cell", "switch to that sheet" is killing your code. You could speed things up by [avoid using select](https://stackoverflow.com/questions/10714251/how-to-avoid-using-select-in-excel-vba). – Darren Bartrup-Cook Oct 16 '18 at 09:14
  • Also your DIM (`Dim drzava$, nov As Workbook, ime$, v%, n%, a As Double`) Is wrong. You should give each variable a type. So: `Dim drzava$ as Workbook, nov as Workbook etc. etc.` – Luuklag Oct 16 '18 at 09:19
  • 1
    `v` & `n` would be better a `Long` rather than `Integer`. They'll throw an overflow error if your worksheet has too many rows. – Darren Bartrup-Cook Oct 16 '18 at 09:19
  • @Luuklag [The special symbols used are shorthand for just that](https://stackoverflow.com/questions/16454621/what-does-a-percentage-symbol-mean-as-part-of-a-variable-name). It is rather opaque though. – eirikdaude Oct 16 '18 at 09:21
  • 1
    Nearly forgot - if the code's working the question is better suited to [Code Review](https://codereview.stackexchange.com/) than here. – Darren Bartrup-Cook Oct 16 '18 at 09:34
  • Thank you all for the answers and for pointing me in the directions where my problems may lay. I will look into it and report on the progress. – Iva Oliva Oct 16 '18 at 09:42

1 Answers1

0

As a start you should try avoiding using select and active-whatever as much as possible. You should also try qualifying what objects you are working on as explicitly as possible, especially when working with several different workbooks at once. For instance, are you certain about what your are doing in these lines, and are you certain that you are doing the operation on the right ranges? Cause I am not, although some of that may be because I don't have the workbooks in front of me.

Cells.Select
Selection.Copy
Selection.PasteSpecial Paste:=xlPasteValues, Operation:=xlNone, SkipBlanks _
    :=False, Transpose:=False
Range("A1").Select

I guess it may be about removing formatting or something? If so there are are probably more efficient ways to do it.

Finally it can be beneficial to turn off some properties when running code, in order to speed it up. I often use these small macros at the start and end of whatever code I am running:

Sub deactivate()
  Application.EnableEvents = False
  Application.ScreenUpdating = False
  Application.DisplayStatusBar = False
  Application.Calculation = xlCalculationManual
End Sub

Sub reaktiver()
  Application.EnableEvents = True
  Application.ScreenUpdating = True
  Application.DisplayStatusBar = True
  Application.Calculation = xlCalculationAutomatic
End Sub

Note that if you are dependent on calculations being done while your code is running, you will have to explicitly do this while Application.Calculation is set to xlCalculationManual.

Furthermore I see that you printed some stuff to the statusbar while your code is running, and if Application.DisplayStatusBar is false this will not display. You will have to make a decision on whether having the information displayed outweighs the extra speed you get by having that property set to false.

There are lots of articles on how to optimize VBA code on the web, for further information on what properties it may be a good idea to turn off, and what it will do, you could for instance have a look here.

eirikdaude
  • 3,106
  • 6
  • 25
  • 50
  • Thank you for your answer. Here is in detail what I am trying to do. I have a overview of 38 subjects, which I can review with the help of listbox. To do that I have many formulas on this particular list to call from the larger datatable. The first bit of code (i thought) copies and pastes as values the whole sheet. Thank you for the small macro suggestions. I used screenupdating before, but not the rest. I will try it and also look into the optimization of the code. – Iva Oliva Oct 16 '18 at 09:35