Sluggish Performance when using ranges from two workbooks to update the one

Question

I'm hoping someone can help me here, I'm working with a file which has about 38000 rows and 180 columns. I'm using a macro to update the fields in the one workbook with the values in the other workbook, but it takes about 2 minutes to run. I'm looking for a way to reduce this time, I've tried everything I could find on previous questions but it's still too long.

As you can see in the code below, the macro checks to see the # of rows are the same in each workbook (note the one has 1 more row, hence the lastrow temp having +1) and then I want to check if the field in the temp file is a certain colour, if not then change it if etc... I use this colour to keep track of the values that have changed from the raw file, as I don't want these values to be overwritten again once they have been changed once. I use ranges so that I don't need to access the worksheets the entire time as this increases the execution time. Any help will be appreciated.

Sub SavetoTemp()

    StartTime = Timer
    Set wb = ThisWorkbook
    Set DT = wb.Worksheets("Data Table")

     With Application
        .Calculation = xlCalculationManual
        .ScreenUpdating = False
        .DisplayStatusBar = False
        .EnableEvents = False
        .ActiveSheet.DisplayPageBreaks = False
        DT.DisplayPageBreaks = False
    End With

    Dim TempFile As Workbook
    Dim TempSheet As Worksheet
    Dim LastCol As Long
    Dim colLetEnd As String

    If wb.Worksheets("Steering Wheel").Range("M30").Value = "" Then
        MsgBox "Please Select a Temporary File First"
        Exit Sub
    Else
        Set TempFile = Workbooks.Open(Range("M30").Value)
        Set TempSheet = TempFile.Worksheets(1)

        Dim LastRowDT As Long
        LastRowDT = DT.Cells(Rows.Count, "A").End(xlUp).row
        LastCol = DT.Cells(1, Columns.Count).End(xlToLeft).Column

        Dim LastRowTemp As Long
        LastRowTemp = TempSheet.Cells(Rows.Count, "A").End(xlUp).row + 1

        Dim tempCell As Range
        Dim r As Long
        Dim c As Long
        Dim rngDT As String
        Dim rngTemp As Range
        colLetEnd = Split(Cells(1, LastCol).Address, "$")(1)
        Set rngTemp = TempSheet.UsedRange
        rngDT = "A" & 3 & ":" & colLetEnd & LastRowDT

        If (LastRowTemp = LastRowDT) Then
            For Each cell In DT.Range(rngDT)
                Set tempCell = rngTemp.Cells(cell.row - 1, cell.Column)
                If Not tempCell.Interior.Color = RGB(188, 146, 49) Then
                    If IsNumeric(cell) And Not IsEmpty(cell) Then
                        If (Not cell = tempCell) Or (IsEmpty(tempCell)) Then
                            tempCell.Interior.Color = RGB(188, 146, 49)
                            tempCell = cell
                        End If
                    Else
                        If Not (StrComp(cell, tempCell, vbTextCompare) = 0) Then
                            tempCell.Interior.Color = RGB(188, 146, 49)
                            tempCell = cell
                        End If
                        End If
                    End If
            Next cell


            TempSheet.Cells.EntireColumn.AutoFit
            TempFile.Save
            TempFile.Close

            MsgBox "All Records Saved to Temp File Successfully!"
            wb.Worksheets("Steering Wheel").Activate
            MinutesElapsed = Format((Timer - StartTime) / 86400, "hh:mm:ss")
            wb.Worksheets("Steering Wheel").Range("E48").Value = MinutesElapsed
            With Application
                .Calculation = xlCalculationAutomatic
                .ScreenUpdating = True
                .DisplayStatusBar = True
                .EnableEvents = True
            End With
            Else
            MsgBox "Please load the raw data file into your temp file before saving to it."
            With Application
                .Calculation = xlCalculationAutomatic
                .ScreenUpdating = True
                .DisplayStatusBar = True
                .EnableEvents = True
            End With
            Exit Sub
        End If
    End If
End Sub

Any improvement to the code to reduce execution time is my ultimate goal.

Answer 1

I will start off with an advice on performance and then comment on a few more general topics.

Performance

Your general performance problem is probably that you make too many sheet accesses. These are slow. Instead, you could read in all values in a range in one go. Used on a range containing more than a single cell, the Value property returns a Variant containing a 2-dim array of the values in the first area of the range.

You could simply read in the values of both ranges, do the comparison and write the value from the data sheet if the values do not agree and your flag colour is not set. Unfortunately, you cannot avoid the single cell accesses for writing because you save whether the value is to be updated via the cell colour.

Unqualified Member Calls

Please note that every unqualified use of the Cells member does resolve to ActiveSheet.Cells. The same is true for Rows and Columns. For your code, this does not really make a difference since you use these methods to calculate things that do not depend on a specific worksheet. However, it looks like an error at first.

Use of Default Members

Using default members to shorten the code, usually is good for readability. Any future maintainer of the code will thank you if you are more explicit about what you do. Thus, it would be helpful to replace the implicit default member calls in statements like tempCell = cell with the equivalent version tempCell.Value = cell.Value.

Code Names for Sheet Access

You can change the code name of sheets in the workbook containing the code in the properties window for the sheet. These names allow you to access the sheet directly via this name without requiring the stringly-typed access via Worksheets. E.g. if you set the code name of the sheet named "Steering Wheel" to SteeringWheel, you can access cell M30 via SteeringWheel.Range("M30").

Avoiding Magic Numbers

In general, using the same explicit number in several places inside a program is a mistake waiting to happen. When the value needs to be changed later, one has to find all occurrences in the code and replace them all. To avoid this, it makes sense to give the value a name and save it in one place, e.g. as a constant. Here, this applies to the colour used as marker colour.

Named Ranges

Accessing data from a worksheet via its address means that whenever the layout changes, the code has to be adapted as well. This can be avoided using named ranges. If you introduced a named range TempFilePathRange for cell M30 scoped to the steering wheel worksheet, you could get the file path via SteeringWheel.Range("TempFilePathRange").Value, which does not only work after layout changes, but also conveys much better what it does.

Single Responsibility Principle

For maintainability, it is generally a good idea to follow the single responsibility principle (SRP), which basically says that each method should be responsible for one and only one thing. The hard thing about it is to find a good definition of responsibility for the given situation.

You procedure definitely has a lot of responsibilities: Disabling and enabling all automatic updates, getting the temporary file path, opening and closing the temporary file, getting the data worksheet, extracting the ranges to compare and executing the comparison.

You could split this into several procedures and functions. The outermost could simply do the enabling and disabling of Excel features and call a private procedure doing the comparison in between. The inner one could call a function that returns a reference to the temporary file (and Nothing if it does not exist), which calls another function responsible for determining the file name. Next, it could call a function that extracts the range used for comparison from the temporary file and one that returns the reference range. Then these could be sent to a procedure executing the actual comparisons and updates. Finally, the procedure would close the temporary file. Similarly, the comparison operation could also e extracted into its own function.

After such a split, each sub-procedure or function would do a short sequence of simple actions with the details hidden in the functions and procedures it calls. This is generally much easier to digest then having to deal with all details at once. In addition, this approach this basically documents you code automatically, provided you use meaningful names for the functions and procedures.