-2

Please see the below VBA Code that I've came up with. Essentially, this is to open another workbook, Unmerge the Rows, Copy the Columns and Paste it into my Active Workbook. However after copy pasting, when the code runs to the CalculationAutomatic line, it takes around 15mins. Is there any other way to make it more efficient? Thank you

Option Explicit

Sub ImportRemarks()
Application.Calculation = xlCalculationManual
Application.ScreenUpdating = False

Dim PLPath As String
PLPath = Sheets("Instructions").Range("C16").Text
Dim wbThis As Workbook
Dim wbTarget As Workbook
Set wbThis = ActiveWorkbook
Set wbTarget = Workbooks.Open(PLPath)

wbTarget.Worksheets("Performance List").Select
Rows("1:2").Select
Selection.UnMerge
wbThis.Worksheets("keys").Range("I:I").Value = 
wbTarget.Worksheets("Performance List").Range("F:F").Value
wbThis.Worksheets("keys").Range("J:L").Value = 
wbTarget.Worksheets("Performance List").Range("P:R").Value
wbThis.Activate
Application.CutCopyMode = False
wbTarget.Close savechanges:=False

ActiveWorkbook.Sheets("Instructions").Select
Range("C22").Select
Application.Calculation = xlCalculationAutomatic
Application.ScreenUpdating = True

End Sub
Dan
  • 45,079
  • 17
  • 88
  • 157
ya23
  • 53
  • 1
  • 1
  • 6
  • 2
    [Get rid of .Select to start with](https://stackoverflow.com/questions/10714251/how-to-avoid-using-select-in-excel-vba) – QHarr Aug 29 '18 at 12:13
  • 2
    The above has typos and is missing line breaks btw. E.g. Applicaation.Calculation = xlCalculationAutomatic So it shouldn't run at all. Correct those and see what happens. – QHarr Aug 29 '18 at 12:16
  • But the issue is, when it runs to xlCalculationAutomatic, it takes a long time to process – ya23 Aug 29 '18 at 12:33
  • @Zy23 then it sounds like this VBA is not your problem at all, but that your workbook it self is just very slow. Do you have a lot of [volatile functions](http://www.decisionmodels.com/calcsecretsi.htm) in your workbook? Or maybe a lot of `VLOOKUP` or `MATCH` over very large ranges? – Dan Aug 29 '18 at 12:37
  • Should your initial ActiveWorkbook referenced here: Set wbThis = ActiveWorkbook be ThisWorkbook? i.e. the workbook containing the code? – QHarr Aug 29 '18 at 12:40
  • @Dan Yes it has a lot of volatile functions but then I've another code which copies a larger set of data than this but it takes 3-4mins to run. The code is similar to the one above. – ya23 Aug 29 '18 at 12:42
  • @QHarr Yes the ActiveWorkbook is containing the code – ya23 Aug 29 '18 at 12:43
  • @Zy23 perhaps you can paste over some of those functions as values? Either keep the bottom few rows as formulae in case you need to expand them down, or else use VBA to paste the formulae in (using R1C1 notation) and then paste them as values... – Dan Aug 29 '18 at 12:55

1 Answers1

1

Maybe something like as follows for starters. Ideally, the optimization steps would go in their own subs. One to switch on optimization at the start and the other to return everything to how it was at the end (or on error).

As requested, this shows you how to remove the .Select parts of your code by using With statements. It also includes a safe exit, in case of error, to switch back on everything you disabled.

Option Explicit
Public Sub ImportRemarks()
    Dim PLPath As String, wbThis As Workbook, wbTarget As Workbook

    Application.Calculation = xlCalculationManual
    Application.ScreenUpdating = False
    Application.EnableEvents = False

    On Error GoTo Errhand
    Set wbThis = ThisWorkbook
    Set wbTarget = Workbooks.Open(PLPath)
    PLPath = wbThis.Worksheets("Instructions").Range("C16").Text

    wbTarget.Worksheets("Performance List").Rows("1:2").UnMerge

    With wbThis.Worksheets("keys")
        .Range("I:I") = wbTarget.Worksheets("Performance List").Range("F:F")
        .Range("J:L") = wbTarget.Worksheets("Performance List").Range("P:R")
    End With

    wbTarget.Close savechanges:=False

    With wbThis
        .Activate
        ' .Worksheets("Instructions").Range("C22").Activate '<=consider whether this is needed?
    End With

Errhand:
    Application.Calculation = xlCalculationAutomatic
    Application.ScreenUpdating = True
    Application.EnableEvents = True
End Sub

More info on optimization here:

https://www.thespreadsheetguru.com/blog/2015/2/25/best-way-to-improve-vba-macro-performance-and-prevent-slow-code-execution

http://www.cpearson.com/excel/optimize.htm

QHarr
  • 83,427
  • 12
  • 54
  • 101
  • 1
    Hummm.... a downvote for being helpful. Would the downvoter care to comment on how the above is not helpful? – QHarr Aug 29 '18 at 13:05