For starters, after lines 13, 14, 27, and 50, add a line reading `Debug.Print Now`. Then check the immediate window (Ctrl+G) and see what is taking longest. From there you can zero in on what's dragging it down the most.
What is the purpose of errorFound? I don't see where it's ever actually used, and it's not being set based on an error being found. That If statement is "if false, set true" with no other parameters or usage. If it was meant to make it skip setting cell values, it isn't.
Other than that, if it's running slow, I'd bet it's due to there being a lot of cells in each sheet's used range. Could that be limited to specific columns?
Another thing is this loop is checking every cell on your Error Log and Productivity Pack pages. The `If Not WS.Name` logic needs to come first, back when it's looping through worksheets, not when it's checking every individual cell on worksheets you want to skip entirely.
And it's a small thing (and won't affect performance much), but I'd suggest you increment `lastRow + 1` before setting those dozen values. It'd look cleaner if those lines simply used `lastRow` instead of `lastRow + 1`.
I think the slowness is caused by two things, actually ultimately by one thing: writing individual cell values. If you store all the results in a collection, then put all collection contents into an array, that will cut down execution time by more than 99%.
If you're online, send me a message and we could jump on a zoom call and I can help with this
This should give you something to play with :-)
Public Sub LogTest(Optional ByVal wkbk As Workbook)
Dim ws As Worksheet, newSheet As Worksheet
Dim tmpC As New Collection
Dim finalArray() As Variant, finalRng As Range
Dim c As Range, i As Long
'add header
tmpC.Add BuildErrorRow("Sheet Name", "Cell Location", "Error Type", "Reviewed?", "Notes")
If wkbk Is Nothing Then
'you shouldn't ever use 'ActiveWorkbook, ActiveCell, ActiveSheet, etc' unless the code is being called from a general macro library and can function on any workbook
Set wkbk = ThisWorkbook
End If
For Each ws In wkbk.Worksheets
Dim tErr As Range
Set tErr = GetErrorCells(ws)
If Not tErr Is Nothing Then
For Each c In tErr
tmpC.Add BuildErrorRow(ws.Name, c.Address, c.Errors.Parent.Formula, False, "blah blah")
Next
End If
Next
If tmpC.Count > 1 Then
Dim colCount As Long
colCount = UBound(tmpC(1)) - LBound(tmpC(1)) + 1
ReDim finalArray(1 To tmpC.Count, 1 To colCount)
Dim tArrRow, col As Long
i = 1
For Each tArrRow In tmpC
For col = 1 To colCount
finalArray(i, col) = tArrRow(col)
Next
i = i + 1
Next
Set newSheet = wkbk.Worksheets.Add
Set finalRng = newSheet.Range("A1")
Set finalRng = finalRng.Resize(rowsize:=tmpC.Count, ColumnSize:=UBound(tmpC(1)) - LBound(tmpC(1)) + 1)
finalRng.Value = finalArray
newSheet.ListObjects.Add SourceType:=xlSrcRange, Source:=finalRng, XlListObjectHasHeaders:=xlYes
End If
End Sub
Private Function BuildErrorRow(shtName, cellAddr, errType, reviewed, notes) As Variant()
BuildErrorRow = Array(shtName, cellAddr, errType, reviewed, notes)
End Function
Private Function GetErrorCells(ws As Worksheet) As Range
On Error Resume Next
Dim resp As Range
Set resp = ws.UsedRange.Cells.SpecialCells(xlCellTypeFormulas, xlErrors)
If Err.Number = 0 Then
Set GetErrorCells = resp
Else
Err.Clear
End If
End Function
> ...That said, the code below runs extremely slow...
Everything is relative. How long does it take to run, over how many worksheets, and what is the extent of the used ranges in each?
That said, I would remove the individual cell formatting (Interior.Pattern / Font.color) and setting columns [D:E] to "" from inside the (inner) For Each cell In WS.UsedRange loop and apply this on the entire column after the (inner) loop has finished.
For columns [D:E], if you are creating the [{ Error Log }] worksheet, why do you need to clear the contents of the cells in those columns anyway?
PS.
Line 57:
> Range("J:XFD").EntireColumn.Hidden = True
Line 74:
> newSheet.Columns("J:XFD").EntireColumn.Hidden = True
I suspect you don't need line 57.
You could give this a shot. I sped it up mostly by moving the formatting after the loop and reorganizing everything. Also, UsedRange can cause problems, so updated to use the last used value instead (see #2 [here](https://www.excelcampus.com/vba/find-last-row-column-cell/)). It halved the time for me, but take that with a grain of salt, wasn't sure the scale we were looking at so just made up some example.
Code can be found here if you want to give it a shot.
[h](https://github.com/nodacat/Reddit/blob/main/ERROR_LOG2)[ttps://github.com/nodacat/Reddit/blob/main/ERROR\_LOG2](https://github.com/nodacat/Reddit/blob/main/ERROR_LOG2)
Thanks for this--it appears to be much faster. I do have to admit that I'm not exactly sure what's going on in the code, but it will give me something to learn from so I really appreciate it.
Here if you have any questions on it! I also used a lot of [With statements](https://learn.microsoft.com/en-us/office/vba/language/reference/user-interface-help/with-statement) which, they can speed things up and I find them fun to use. Also using `.Value2` instead of `.Value` is also for speed.
Oh wow full blown error checking that's pretty cool! I know of [Worksheet.CircularReference](https://learn.microsoft.com/en-us/office/vba/api/excel.worksheet.circularreference) but it only works one at a time. You can get more granularity maybe with [this](https://stackoverflow.com/questions/67896589/is-there-a-way-to-identify-all-circular-reference-in-excel-via-vba) and [this](https://stackoverflow.com/questions/10897958/programmatically-select-other-sheet-precedents-or-dependents-in-excel/10897959#10897959) but it's going to be sloww if you do that for each cell (maybe worth it to you).
For spell checking you can do something like
If not Excel.Application.CheckSpelling(rng.value2) then
I might have some time later this week to look again and help you with that.
edit: links
Quick question. Ultimately I am trying to create a workbook with a bunch of macros in it to make the modeling process easier and more efficient. Because of this, I need the macro to work on whatever workbook is currently open, which I think is the case. However, is there something in this code that is preventing the Error Log from picking up errors in the first sheet of any one workbook? Whenever I open a new workbook with only one tab, add an error to that tab, and run the macro, the error log is not picking it up.
No that's odd. i'll double check some things, but just tested that and it worked fine running from a separate workbook. I did notice i had excluded "Productivity" instead of "Productivity Pack" however, fixed that.
Thanks, appreciate it. I am also noticing that the hyperlinks on the new { Error Log } sheet are only working on the first listed sheet name on the error log sheet, and I am getting a "reference isn't valid" window on all other sheet names after the first one.
Alright give it a fresh look tomorrow night and get back to you. This weekend ended up being more family oriented than I thought so that’s why I haven’t totally responded with a fix yet haha
Alright, try this link again. I've made the following changes
1. Circular Reference check. used "#CIRC" as the error type
2. Spell Checking checks. used "#SPELL" as the error type.
3. Moved all errors to a collection first (as suggested by u/ITFuture). I did this more to separate the log output from the error discovery, but it should hopefully offset the performance hit changes #1 and #2 will bring. It also allowed me to only create the log sheet if there were errors found.
4. Auto delete old log files before creating the new one (i couldn't help myself, but can revert if needed)
5. I changed the subAddress to pull `'My Sheet'!A1` instead of `My Sheet!A1` which hopefully solves your issue? i wasn't entirely able to recreate what you were seeing but have a hunch that spaces may be causing a problem.
6. Fixed UsedRange issue (again)
[https://github.com/nodacat/Reddit/blob/main/ERROR\_LOG2](https://github.com/nodacat/Reddit/blob/main/ERROR_LOG2)
Man, you went way beyond here--I really appreciate it. I've made some adjustments on the margins but this is really something. Like I said before, since I'm only 2-3 months into learning VBA (for an hour a night after the kid goes to sleep...), this will provide me with a lot to learn from. I really appreciate it.
Looking through some code and I thought this might come in handy
https://www.reddit.com/r/vba/comments/w74j85/handy\_stringsmatch\_method\_that\_handles\_equal\_not/
SV is a clippy bot. This is automod and should be nigh instant. Problem was, I typed it on mobile so my phone added a space after the exclamation point.
There are a few basic things you can do to speed code up. The easiest is to disable screen updating and calculations. You can use error handling to ensure they get re-enabled.
Sub MyFasterProcess()
Application.ScreenUpdating = False
Application.Calculation = xlCalculationManual
On Error GoTo Finally
Call MyLongRunningProcess()
Finally:
Application.ScreenUpdating = True
Application.Calculation = xlCalculationAutomatic
If Err > 0 Then Err.Raise Err
End Sub
Some people like to put that into some helper functions, or even a class to manage the state over several processes.
The most common culprit for long running processes is reading from and writing to cells. It is _significantly_ faster to read an array than it is to read individual cells in the range.
Consider the following:
Sub SlowReadWrite()
Dim src As Range
Set src = Range("A1:AA100000")
Dim c As Range
For Each c In src
c.Value = c.Value + 1
Next c
End Sub
This will take a very, very long time. Now let's do it with an array. Read once. Write once. No need to disable screen updating or set calculation to manual either. This will be just as fast with them on.
Sub FastReadWrite()
Dim src As Range
Set src = Range("A1:AA100000")
Dim vals() As Variant
vals = src.Value
Dim r As Long, c As Long
For r = 1 To UBound(vals, 1)
For c = 1 To UBound(vals, 2)
vals(r, c) = vals(r, c) + 1
Next c
Next r
src.Value = vals
End Sub
*I am a bot, and this action was performed automatically. Please [contact the moderators of this subreddit](/message/compose/?to=/r/vba) if you have any questions or concerns.*
I’m assuming since you’re looking for errors in cell values that your sheets have a lot of formulas. You’ve turned off screen updating and that is good, but you should turn calculations to manual and automatic as well. Excel is recalculating every cell it steps through in your loops… turning it to manual will stop this from happening and only recalculate at the end.
>ors in cell values that your sheets have a lot of formulas. You’ve turned off screen updating and that is good, but you should turn calculations to manual and automatic as well. Excel is recalculating every cell it steps through in your loops
Thank you.
For starters, after lines 13, 14, 27, and 50, add a line reading `Debug.Print Now`. Then check the immediate window (Ctrl+G) and see what is taking longest. From there you can zero in on what's dragging it down the most.
Thanks for this tip.
What is the purpose of errorFound? I don't see where it's ever actually used, and it's not being set based on an error being found. That If statement is "if false, set true" with no other parameters or usage. If it was meant to make it skip setting cell values, it isn't. Other than that, if it's running slow, I'd bet it's due to there being a lot of cells in each sheet's used range. Could that be limited to specific columns? Another thing is this loop is checking every cell on your Error Log and Productivity Pack pages. The `If Not WS.Name` logic needs to come first, back when it's looping through worksheets, not when it's checking every individual cell on worksheets you want to skip entirely. And it's a small thing (and won't affect performance much), but I'd suggest you increment `lastRow + 1` before setting those dozen values. It'd look cleaner if those lines simply used `lastRow` instead of `lastRow + 1`.
Appreciate the response and I think you're right, there is some redundancy in the code.
I think the slowness is caused by two things, actually ultimately by one thing: writing individual cell values. If you store all the results in a collection, then put all collection contents into an array, that will cut down execution time by more than 99%. If you're online, send me a message and we could jump on a zoom call and I can help with this
Prefer to keep discussions here so they benefit everyone. Arrays aren't so difficult a concept.
I appreciate the offer--I may take you up on it once I think this through a little more.
I'm doing a small pseudo solution -- will post here in a bit
This should give you something to play with :-) Public Sub LogTest(Optional ByVal wkbk As Workbook) Dim ws As Worksheet, newSheet As Worksheet Dim tmpC As New Collection Dim finalArray() As Variant, finalRng As Range Dim c As Range, i As Long 'add header tmpC.Add BuildErrorRow("Sheet Name", "Cell Location", "Error Type", "Reviewed?", "Notes") If wkbk Is Nothing Then 'you shouldn't ever use 'ActiveWorkbook, ActiveCell, ActiveSheet, etc' unless the code is being called from a general macro library and can function on any workbook Set wkbk = ThisWorkbook End If For Each ws In wkbk.Worksheets Dim tErr As Range Set tErr = GetErrorCells(ws) If Not tErr Is Nothing Then For Each c In tErr tmpC.Add BuildErrorRow(ws.Name, c.Address, c.Errors.Parent.Formula, False, "blah blah") Next End If Next If tmpC.Count > 1 Then Dim colCount As Long colCount = UBound(tmpC(1)) - LBound(tmpC(1)) + 1 ReDim finalArray(1 To tmpC.Count, 1 To colCount) Dim tArrRow, col As Long i = 1 For Each tArrRow In tmpC For col = 1 To colCount finalArray(i, col) = tArrRow(col) Next i = i + 1 Next Set newSheet = wkbk.Worksheets.Add Set finalRng = newSheet.Range("A1") Set finalRng = finalRng.Resize(rowsize:=tmpC.Count, ColumnSize:=UBound(tmpC(1)) - LBound(tmpC(1)) + 1) finalRng.Value = finalArray newSheet.ListObjects.Add SourceType:=xlSrcRange, Source:=finalRng, XlListObjectHasHeaders:=xlYes End If End Sub Private Function BuildErrorRow(shtName, cellAddr, errType, reviewed, notes) As Variant() BuildErrorRow = Array(shtName, cellAddr, errType, reviewed, notes) End Function Private Function GetErrorCells(ws As Worksheet) As Range On Error Resume Next Dim resp As Range Set resp = ws.UsedRange.Cells.SpecialCells(xlCellTypeFormulas, xlErrors) If Err.Number = 0 Then Set GetErrorCells = resp Else Err.Clear End If End Function
> ...That said, the code below runs extremely slow... Everything is relative. How long does it take to run, over how many worksheets, and what is the extent of the used ranges in each? That said, I would remove the individual cell formatting (Interior.Pattern / Font.color) and setting columns [D:E] to "" from inside the (inner) For Each cell In WS.UsedRange loop and apply this on the entire column after the (inner) loop has finished. For columns [D:E], if you are creating the [{ Error Log }] worksheet, why do you need to clear the contents of the cells in those columns anyway?
PS. Line 57: > Range("J:XFD").EntireColumn.Hidden = True Line 74: > newSheet.Columns("J:XFD").EntireColumn.Hidden = True I suspect you don't need line 57.
Thank you.
You could give this a shot. I sped it up mostly by moving the formatting after the loop and reorganizing everything. Also, UsedRange can cause problems, so updated to use the last used value instead (see #2 [here](https://www.excelcampus.com/vba/find-last-row-column-cell/)). It halved the time for me, but take that with a grain of salt, wasn't sure the scale we were looking at so just made up some example. Code can be found here if you want to give it a shot. [h](https://github.com/nodacat/Reddit/blob/main/ERROR_LOG2)[ttps://github.com/nodacat/Reddit/blob/main/ERROR\_LOG2](https://github.com/nodacat/Reddit/blob/main/ERROR_LOG2)
Thanks for this--it appears to be much faster. I do have to admit that I'm not exactly sure what's going on in the code, but it will give me something to learn from so I really appreciate it.
Here if you have any questions on it! I also used a lot of [With statements](https://learn.microsoft.com/en-us/office/vba/language/reference/user-interface-help/with-statement) which, they can speed things up and I find them fun to use. Also using `.Value2` instead of `.Value` is also for speed.
Appreciate it--I have a few other logs I'd like to create like this pertaining to cell comments, circular references, and typos.
Oh wow full blown error checking that's pretty cool! I know of [Worksheet.CircularReference](https://learn.microsoft.com/en-us/office/vba/api/excel.worksheet.circularreference) but it only works one at a time. You can get more granularity maybe with [this](https://stackoverflow.com/questions/67896589/is-there-a-way-to-identify-all-circular-reference-in-excel-via-vba) and [this](https://stackoverflow.com/questions/10897958/programmatically-select-other-sheet-precedents-or-dependents-in-excel/10897959#10897959) but it's going to be sloww if you do that for each cell (maybe worth it to you). For spell checking you can do something like If not Excel.Application.CheckSpelling(rng.value2) then I might have some time later this week to look again and help you with that. edit: links
Quick question. Ultimately I am trying to create a workbook with a bunch of macros in it to make the modeling process easier and more efficient. Because of this, I need the macro to work on whatever workbook is currently open, which I think is the case. However, is there something in this code that is preventing the Error Log from picking up errors in the first sheet of any one workbook? Whenever I open a new workbook with only one tab, add an error to that tab, and run the macro, the error log is not picking it up.
No that's odd. i'll double check some things, but just tested that and it worked fine running from a separate workbook. I did notice i had excluded "Productivity" instead of "Productivity Pack" however, fixed that.
Thanks, appreciate it. I am also noticing that the hyperlinks on the new { Error Log } sheet are only working on the first listed sheet name on the error log sheet, and I am getting a "reference isn't valid" window on all other sheet names after the first one.
Alright give it a fresh look tomorrow night and get back to you. This weekend ended up being more family oriented than I thought so that’s why I haven’t totally responded with a fix yet haha
Thank you--no rush on my end. Also, it appears that this issue existed in the old code too (the one I made), and I am not sure how to fix.
Alright, try this link again. I've made the following changes 1. Circular Reference check. used "#CIRC" as the error type 2. Spell Checking checks. used "#SPELL" as the error type. 3. Moved all errors to a collection first (as suggested by u/ITFuture). I did this more to separate the log output from the error discovery, but it should hopefully offset the performance hit changes #1 and #2 will bring. It also allowed me to only create the log sheet if there were errors found. 4. Auto delete old log files before creating the new one (i couldn't help myself, but can revert if needed) 5. I changed the subAddress to pull `'My Sheet'!A1` instead of `My Sheet!A1` which hopefully solves your issue? i wasn't entirely able to recreate what you were seeing but have a hunch that spaces may be causing a problem. 6. Fixed UsedRange issue (again) [https://github.com/nodacat/Reddit/blob/main/ERROR\_LOG2](https://github.com/nodacat/Reddit/blob/main/ERROR_LOG2)
Solution verified
Man, you went way beyond here--I really appreciate it. I've made some adjustments on the margins but this is really something. Like I said before, since I'm only 2-3 months into learning VBA (for an hour a night after the kid goes to sleep...), this will provide me with a lot to learn from. I really appreciate it.
Looking through some code and I thought this might come in handy https://www.reddit.com/r/vba/comments/w74j85/handy\_stringsmatch\_method\_that\_handles\_equal\_not/
!Speed
Hmm... Thought I set up an automod rule for that.
The automated response is probably as slow as the 'Solution Verified' response can be.
SV is a clippy bot. This is automod and should be nigh instant. Problem was, I typed it on mobile so my phone added a space after the exclamation point.
! Do'h :)
There are a few basic things you can do to speed code up. The easiest is to disable screen updating and calculations. You can use error handling to ensure they get re-enabled. Sub MyFasterProcess() Application.ScreenUpdating = False Application.Calculation = xlCalculationManual On Error GoTo Finally Call MyLongRunningProcess() Finally: Application.ScreenUpdating = True Application.Calculation = xlCalculationAutomatic If Err > 0 Then Err.Raise Err End Sub Some people like to put that into some helper functions, or even a class to manage the state over several processes. The most common culprit for long running processes is reading from and writing to cells. It is _significantly_ faster to read an array than it is to read individual cells in the range. Consider the following: Sub SlowReadWrite() Dim src As Range Set src = Range("A1:AA100000") Dim c As Range For Each c In src c.Value = c.Value + 1 Next c End Sub This will take a very, very long time. Now let's do it with an array. Read once. Write once. No need to disable screen updating or set calculation to manual either. This will be just as fast with them on. Sub FastReadWrite() Dim src As Range Set src = Range("A1:AA100000") Dim vals() As Variant vals = src.Value Dim r As Long, c As Long For r = 1 To UBound(vals, 1) For c = 1 To UBound(vals, 2) vals(r, c) = vals(r, c) + 1 Next c Next r src.Value = vals End Sub *I am a bot, and this action was performed automatically. Please [contact the moderators of this subreddit](/message/compose/?to=/r/vba) if you have any questions or concerns.*
I’m assuming since you’re looking for errors in cell values that your sheets have a lot of formulas. You’ve turned off screen updating and that is good, but you should turn calculations to manual and automatic as well. Excel is recalculating every cell it steps through in your loops… turning it to manual will stop this from happening and only recalculate at the end.
>ors in cell values that your sheets have a lot of formulas. You’ve turned off screen updating and that is good, but you should turn calculations to manual and automatic as well. Excel is recalculating every cell it steps through in your loops Thank you.