PDA

View Full Version : [SOLVED] Out of Stack Space



LordDragon
08-14-2015, 11:16 AM
Greetings,

I have a small problem with my workbook and am looking for any help I can get.

There are 16 Worksheets, most of which have a Worksheet_Change() event.



Private Sub Worksheet_Change(ByVal Target As Range)
'Allows the Yes/No fields to accept partial entries and lower case.


Application.ScreenUpdating = False

'Declare variables
Dim lngRow As Long

'Determine how many rows there are
lngRow = Range("A" & Rows.Count).End(xlUp).Row

'Set the acceptance to include lower case and single letters.
For Each cell In Range("A3:A" & lngRow)
If cell = "y" Then cell.Value = strYes
If cell = "yes" Then cell.Value = strYes
If cell = "n" Then cell.Value = strNo
If cell = "no" Then cell.Value = strNo
Next cell

Application.ScreenUpdating = True

End Sub


There is also a Workbook_Activate() event and 8 Modules. Everything seemed to be working fine, until I made a small change to one of the Worksheet_Change events that simply looked for a value in one cell and if it existed, then it changed the value of two other cells.



If ActiveWorkbook.Worksheets("EDR").Range("A65").Value = "Yes" Then
With ActiveWorkbook.Worksheets("EDR")
.Range("A43").Value = "Yes"
.Range("E43").Value = 1
End With
End If


I don't see how it was causing a problem. But I kept getting an "Out of Stack Space" message.

I did some research and I found this thread (http://www.vbaexpress.com/forum/showthread.php?50830-Worksheet-sheetchange-out-of-stack)posted by SamT (http://www.vbaexpress.com/forum/member.php?6494-SamT) last year to someone else with the same problem. The thread was not marked solved and the original poster never posted anything after SamT. So I don't know if the suggestion worked for him, but I gave it a try with my workbook.



Private Sub Worksheet_Change(ByVal Target As Range, ByVal Sh As Object)
'Allows the Yes/No fields to accept partial entries and lower case.


'Provide a way to watch a secific range for changes (instead of the entire sheet).
Dim RangeToWatch As Range

'Check to see if this sub is already running.
Static MeAlreadyRunning As Boolean

If MeAlreadyRunning Then Exit Sub
MeAlreadyRunning = True

'Check to make sure that this is the sheet that is being watched by the sub.
If Not Sh.Name = "Master Parts List" Then Exit Sub

'Set the range to watch.
Set RangeToWatch = Sh.Range("A3", "A1958")
If Intersect(Target, RangeToWatch) Is Nothing Then Exit Sub

'Disable application level events.
With Application
.EnableEvents = False
.Calculation = xlCalculationManual
.ScreenUpdating = False
End With

'Declare variables.
Dim lngRow As Long

'Determine how many rows there are.
lngRow = Range("A" & Rows.Count).End(xlUp).Row

'Set the acceptance to include lower case and single letters.
For Each cell In Range("A3" & lngRow)
If cell = "y" Then cell.Value = strYes
If cell = "yes" Then cell.Value = strYes
If cell = "n" Then cell.Value = strNo
If cell = "no" Then cell.Value = strNo
Next cell

'Enable the application level events.
With Application
.Calculation = xlCalculationAutomatic
.ScreenUpdating = True
.EnableEvents = True
End With

'Turn off the sub is running event.
MeAlreadyRunning = False


End Sub


Now I'm getting the following message box.
14166

I know my workbook has a lot of code and is trying to do a lot of things, but I've been trying to be careful to not have any one sub try to do too much. Or to have more than one sub doing anything at one time. So I'm looking for any methods I can use to speed up my code and make it work smoother.

I read about using Workbook level constants for things that won't change, so I removed all the duplicated variables and put them in a module as Workbook level constants (unless their value actually changes). This did speed it up a little, but apparently there is still a long way to go.



'Decalre the Constants for the entire workbook.
Public Const strYes As String = "Yes"
Public Const strNo As String = "No"
Public Const strVolt1 As String = "110V"
Public Const strVolt2 As String = "220V"
Public Const strPrbR As String = "Radar"
Public Const strPrbM As String = "Mud"
Public Const strPrbB As String = "Both"


I have also attached a copy of the entire workbook here. This copy does NOT include the code I got from SamT, or my code to check for that value. Basically, it's the last working version before attempting these changes.

I'm not dead set on using any particular method, I'm just looking for guidance on how to make my code smoother. I have a list of requirements for this project and am trying to meet them all. However, the more of them I meet, the slower it runs and the more it seems to want to crash.

p45cal
08-14-2015, 03:20 PM
You need a sprinkling of Application.enableevents=false/true. This is because each change effected by the change event code is itself triggering a fresh change event, ad infinitum.
Do a search on the code in the attachment for enableevents to see where I've added them.
I've also consolidated the repeated Worksheet_change events on 11 sheets to the single workbook_sheetchange event. This means you only have to tweak the code in one place and not have to change it in 11 places!
I think there could be streamlining in some of the other macros too.

LordDragon
08-14-2015, 05:39 PM
That worked great. I figured there was a way to move that Worksheet_Change event to a single sub that would check all the sheets. I just wasn't sure how to go about it.

Thanks for the help.

I just read something about adding that Application.EnableEvents = False/True thing too. It just wasn't clear where to put it and when to use it. I've seen it before and even used it, I just never knew exactly what it did.

Thanks again.

SamT
08-15-2015, 09:06 AM
This style of writing Event subs allows you to work on multiple conditions that may trigger the same Event.


Private Sub Worksheet_Change(ByVal Target As Range)
'Only one cell at a time is "active" for the purpose of this Event

With Application
'This affects ALL programming ran until this Event sub terminates at GracefulExit below.
'It is application wide in scope
.ScreenUpdating = False
.EnableEvents = False
End With

If Not Intersect (Target, Range("A:A")) Then
'Checking only the used range of the column is slower in this usage.
'Of course, if multiple conditions exist in a large range, then you must check smaller ranges.
FixYesNo Target
GoTo GracefulExit
End If

'If Not Second_Condition Then
' SecondSub Target 'Note that Target.Address is a String in the form "$A$1".
' GoTo GracefulExit
'End If

GracefulExit:
With Application
.ScreenUpdating = True
.EnableEvents = True
End With
End Sub


Sub FixYesNo(Cel As Range)
'Allows the Yes/No fields to accept partial entries and lower case.

'Faster when many options are required
Select Case LCase(Cel.Value)
'Put most frequently encountered first
Case is = "y": Cel.Value = strYes
Case Is = "n": Cel.Value = strNo
Case Is = "yes": Cel.Value = strYes
Case Is= "no" : Cel.Value = strNo
End Sub


Sub SecondSub(Cel As Range()
'Does something else
End Sub

LordDragon
08-16-2015, 03:41 AM
This style of writing Event subs allows you to work on multiple conditions that may trigger the same Event.

Thanks SamT. I'm going to look further into this.