Solved: two routines under one event, but they don't work?

**andrewvanmar** · 07-26-2007, 02:26 AM

I copied the below code from an example to add multiple routines under a single event: but this one doesn't work (unlike others I made), maybe it's because it's in the workbook object instead of a sheet object?

The error I get is: Compile error: variable not defined.

It points at the word "target"

The code is entered in "this workbook"

[VBA]Private Sub Workbook_Open()

Application.ScreenUpdating = False
Application.EnableEvents = False

eventproc10 Target
eventproc11 Target

Application.EnableEvents = True
Application.ScreenUpdating = True

End Sub

Private Sub eventproc10()
End Sub
Run "ShowAll"
End Sub
'part of the hiding worksheets code
Private Sub eventproc11()
Call disable_menu_items
End Sub
'This code is to prevent mailing from the workbook[/VBA]

This one does work for example, it's on a specific worksheet.

[VBA]Private Sub Worksheet_Change(ByVal Target As Range)

Application.ScreenUpdating = False
Application.EnableEvents = False

eventproc4 Target
eventproc5 Target

Application.EnableEvents = True
Application.ScreenUpdating = True

End Sub

Private Sub eventproc4(ByVal Target As Range)
If Not Intersect(Range("g2"), Target) Is Nothing Then
If IsDate(Target) Then
If Target.Value < Date + 1 Then
MsgBox "The date must be tomorrow or later."
Target.ClearContents
End If
End If
End If
End Sub
Private Sub eventproc5(ByVal Target As Range)
If Not Intersect(Range("G2"), Target) Is Nothing Then

'If Target.Value < Date Then
'If Target.Interior.ColorIndex = 3 Then
Target.Interior.ColorIndex = 36
'End If
'End If
End If
End Sub[/VBA]

**Bob Phillips** · 07-26-2007, 02:36 AM

The cell(s) being processed is passed as a parameter to the worksheet events, which you test in Target.

Workbook_Open doesn't have a Target argument, so you can't use it. You will need to specify the cells you want to work upon. But as the called subs don't have a Range argument, it seems superfluous.

Also best to remove the End Sub before the call to other procs.

**andrewvanmar** · 07-26-2007, 03:11 AM

is that the same case for Workbook_BeforeSave, Workbook_BeforeClose, Workbook_Deactivate and Workbook_WindowActivate ? (they don't seem to work either)

I'm not sure from your explanation how to set up multiple routines from one event then.

could you show me from the above example?

**rory** · 07-26-2007, 03:26 AM

You just need:
[VBA]Private Sub Workbook_Open()

Application.ScreenUpdating = False
Application.EnableEvents = False

eventproc10
eventproc11

Application.EnableEvents = True
Application.ScreenUpdating = True

End Sub
[/VBA]

Regards,
Rory
PS All event procedures have set declarations which cannot be altered. You can find out what they are by using the dropdowns at the top of the main code window in the VB Editor. Different events provide different arguments that you can work with. For example, all Workbook_Before... events procide a Cancel argument that you can use to cancel the event.

**andrewvanmar** · 07-26-2007, 04:50 AM

Hmm, it doesn't whinge about the word target now, but it does point at the ?ventproc10 and say "Argument not optional".
What does that mean?

**rory** · 07-26-2007, 04:56 AM

Interesting choice of phrase....
It means eventproc10 is expecting an argument passed to it, or something it runs is expecting one. I'd guess the latter as what you posted did not require an argument. (I assume you tidied up the End Sub issue?)
Regards,
Rory

**andrewvanmar** · 07-26-2007, 05:27 AM

[vba]

Private Sub Workbook_Open()

Application.ScreenUpdating = False
Application.EnableEvents = False

eventproc10 Target
eventproc11 Target

Application.EnableEvents = True
Application.ScreenUpdating = True

End Sub

Private Sub eventproc10()

Run "ShowAll"

'part of the hiding worksheets code
Private Sub eventproc11()
Call disable_menu_items
End Sub
'This code is to prevent mailing from the workbook

[/vba]

You mean like this?
(btw: i meant whinge (i looked up the other, not what I meant at all. haha. editing that out)

**rory** · 07-26-2007, 05:36 AM

No like this:
[VBA]

Private Sub Workbook_Open()

Application.ScreenUpdating = False
Application.EnableEvents = False

eventproc10
eventproc11

Application.EnableEvents = True
Application.ScreenUpdating = True

End Sub

Private Sub eventproc10()

Run "ShowAll"

'part of the hiding worksheets code
Private Sub eventproc11()
Call disable_menu_items
End Sub [/VBA]

if you still get an error, then either ShowAll or disable_menu_items expects an argument passed to it.

Regards,
Rory

**Bob Phillips** · 07-26-2007, 05:47 AM

BTW, you do know that it is APplication.Run, not Run don't you?

**rory** · 07-26-2007, 06:24 AM

It should work without specifying Application, but I would agree it is better to use it.

**andrewvanmar** · 07-26-2007, 06:30 AM

@ XLD; nope I didn't, but i'm a quick study ( at least that's what I tell myself) so now I do.

@ Rory: saw that I copied the "targets", dumb of me. I'm trying this right away

tested: I get an -expected end sub- after application.run "showall"

**rory** · 07-26-2007, 06:33 AM

Yep, sorry, I missed that. Just type:
End Sub
after that line.

Regards,
Rory

**andrewvanmar** · 07-26-2007, 07:36 AM

I added the end sub, and it works

I'm comparing the code before and after; i don't get it, what is the difference? the spacing is larger, but i'm missing something here since I can't reproduce it on the rest of the code yet

**rory** · 07-26-2007, 07:56 AM

We removed the Target argument that you were passing to the macros you called. Neither of them take an argument, and the Target variable was empty anyway!
Regards,
Rory

**andrewvanmar** · 07-26-2007, 08:01 AM

hmmm, I did that immediatly over the whole set.

would you mind looking at this one then? it's the next one i was trying to fix, but for some (different) reason it doesn't work. It also gives that Argument not optional error.

We (you actually) did something to fix that, but I don't what

[VBA]Private Sub Workbook_BeforeSave(ByVal SaveAsUI As Boolean, Cancel As Boolean)

Application.ScreenUpdating = False
Application.EnableEvents = False

eventproc6
eventproc7

Application.EnableEvents = True
Application.ScreenUpdating = True

End Sub

Private Sub eventproc6(ByVal SaveAsUI As Boolean, Cancel As Boolean)
If IsComplete = False Then
Cancel = True
MsgBox "Cannot save, please fill in all the fields in the orderform."
Else
Cancel = False
End If
'to block saving when certain fields are not filled in
End Sub
Private Sub eventproc7()
If Cancel = True Or bIsClosing = False Then Exit Sub
Application.Run "HideAll"
End Sub
'part of the hiding code[/VBA]

**rory** · 07-26-2007, 08:15 AM

You could do it like this:
[VBA]Private Sub Workbook_BeforeSave(ByVal SaveAsUI As Boolean, Cancel As Boolean)

Application.ScreenUpdating = False
Application.EnableEvents = False

eventproc6 Cancel
eventproc7 Cancel

Application.EnableEvents = True
Application.ScreenUpdating = True

End Sub

Private Sub eventproc6(ByRef Cancel As Boolean)
If iscomplete = False Then
Cancel = True
MsgBox "Cannot save, please fill in all the fields in the orderform."
Else
Cancel = False
End If
'to block saving when certain fields are not filled in
End Sub
Private Sub eventproc7(ByVal Cancel As Boolean)
If Cancel = True Or bisclosing = False Then Exit Sub
Application.Run "HideAll"
End Sub
[/VBA]

but I wouldn't personally, I would put the code in the body of the event routine since it depends on the Cancel argument - something like:
[VBA]Private Sub Workbook_BeforeSave(ByVal SaveAsUI As Boolean, Cancel As Boolean)

Application.ScreenUpdating = False
Application.EnableEvents = False

If iscomplete = False Then
Cancel = True
MsgBox "Cannot save, please fill in all the fields in the orderform."
Else
Cancel = False
End If
If Cancel = False And bIsClosing = True Then Application.Run "HideAll"

Application.EnableEvents = True
Application.ScreenUpdating = True

End Sub
[/VBA]

Regards,
Rory

**andrewvanmar** · 07-26-2007, 08:25 AM

why the changing or to and?

Aside from that, I can do this to the others i mentioned as well?
gonna try immediatly!

**rory** · 07-26-2007, 08:36 AM

No real reason other than it's shorter than:
[VBA]If Cancel = True Or bisclosing = False Then

Else
Application.Run "HideAll"
End If
[/VBA]

Regards,
Rory

**andrewvanmar** · 07-26-2007, 08:57 AM

Ah! see lots to learn *grin*

Let me go through the code as I rearrange things, and then post the book for you to see (if you're curious)

**rory** · 07-26-2007, 09:01 AM

As long as it's still intelligible, shorter = better in my book. Clarity and maintainability are more important than brevity, though.
I would be intrigued to see it...
Rory

Thread: Solved: two routines under one event, but they don't work?

Thread Tools

Display

Solved: two routines under one event, but they don't work?

Posting Permissions