Worksheet excluded doesn't exclude

[davegb]

This is part of some code I wrote to limit acess to some spreadsheets.
It's supposed to ignore the sheet "(Code Key)". But it still asks for a
password.

Private Sub Workbook_SheetChange(ByVal ws As Object, ByVal Target As
Range)
Dim sShName As String

For Each ws In ActiveWorkbook.Worksheets
If Right(ws.Name, 7) <> "Monthly" Then 'ignore these sheeets
Select Case ws.Name
Case "(Code Key)" 'Excluded sheets
Exit Sub

Case Else 'if password hasn't been entered yet, then ask
for password
If bPwrdEntrd = False Then

Call PasswordEntry
End If
End Select
End If
Next
End Sub

I tried using CodeName instead of sheet name, but that didn't work
either. Does anyone see what is wrong?

Thanks again.

 

[NickHK]

Dave,
Why loop through all sheet as ws tells you the sheet involved.
I assume the sheet is actually named "(Code key)", with the brackets ?

Private Sub Workbook_SheetChange(ByVal ws As Object, ByVal Target As Range)

Select Case ws.Name
Case "(Code Key)" 'Excluded sheets
Exit Sub

Case Else 'if password hasn't been entered yet, then ask for
password
If bPwrdEntrd = False Then
Call PasswordEntry
End If
End Select
End Sub

NickHK

 

[davegb]

Dave,
Why loop through all sheet as ws tells you the sheet involved.
I assume the sheet is actually named "(Code key)", with the brackets ?

Private Sub Workbook_SheetChange(ByVal ws As Object, ByVal Target As Range)

Select Case ws.Name
Case "(Code Key)" 'Excluded sheets
Exit Sub

Case Else 'if password hasn't been entered yet, then ask for
password
If bPwrdEntrd = False Then
Call PasswordEntry
End If
End Select
End Sub

NickHK

Thanks for your reply, Nick.

I looped through the sheets because there are actually a lot of sheets
that are excluded. A number are excluded that contain "Monthly" in
their names, somes others excluded individually. I chose "(Code Key)"
just as a test to get the code working, planning to add 3 other
explicit names when I have the code right.

In any case, the part of the code you re-wrote that excludes "(Code
Key)" is the same as mine and doesn't work. That's my question. Why?

 

[Doug Glancy]

Dave,

You should not be using a For Next loop for this. By doing so, you are
testing against every sheet in the workbook, not just the changed sheet.
Get rid of the For Next and it works for me.

It was also confusing to me that you changed the default "ByVal Sh as
Object" to "ws" and then used the ws in your loop. This does not seem like
good practice to me.

hth,

Doug

 

[davegb]

Dave,

You should not be using a For Next loop for this. By doing so, you are
testing against every sheet in the workbook, not just the changed sheet.
Get rid of the For Next and it works for me.

It was also confusing to me that you changed the default "ByVal Sh as
Object" to "ws" and then used the ws in your loop. This does not seem like
good practice to me.

hth,

Doug

Thanks for your reply. Why is this bad practice?

 

[JE McGimpsey]

It's not bad. You can use any variable name as long as it's declared to
be of the correct type.

 

[Doug Glancy]

Dave,

Mainly because I found it confusing (not that hard to do <g>). First, you
changed the default worksheet object name in the event, which for me meant
an extra step of interpretation. Second, as originally defined in the event
arguments, the "ws" is set to the worksheet that has been changed, the
"calling" worksheet. By using it in a For Next loop, you are then setting
"ws" to a series of other values.

So, for me, unless I had a compelling reason to make these two changes, I
wouldn't.

One other thought - I always use "ws" when I'm declaring a worksheet. I
find that inside an event procedure, the sh's stand out and I know that they
are referring to the "calling" sheet and that the ws's refer to the
worksheet object I declared, which helps me keep track of things.

hth,

Doug

 

[davegb]

Thanks for your reply, Doug.

Your reply makes me realize how far over my head I am here. I sort of,
but don't really, get what you're saying here. I thought when I started
this would be fairly simple. I just wanted to password protect some of
the sheets in the workbook, each accessed by a different person with a
different password. The sheet names and passwords are on a hidden sheet
in the workbook. Other sheets in the workbook are password protected
the "conventional way".

So what is the "default worksheet object name in the event"? And what
did I change it to? How? More importantly, what should it be?
The second part I understand a little better. Since "ws" is used in the
event, I shouldn't use it for the loop. So I can change that.

As for the "For next loop", I have that in there to test whether this
is one of the macro protected sheets or one of the conventionally
protected sheets. Are you suggesting that I list all of the worksheets

 

[Doug Glancy]

Hi Dave,

I did not mean to make too much of the "ws" v. "sh" thing. All I meant was
that if you create a WorkSheet_Change event in the VBE by using the dropdown
menus at the top to pick that event, it will start out with an empty
procedure that looks like:

Private Sub Workbook_SheetChange(ByVal Sh As Object, ByVal Target As Range)

I assumed (and I probably shouldn't) that you had started from this and for
some reason changed the "Sh" above - the "default" - to "ws". All I was
thinking is that it would be best to leave it at "Sh". This is certainly
not critical though, and we know that JE, who is far smarter at this, does
not agree.

You said exactly what I was thinking about the second part, regarding not
reusing "ws". Again, this does not actually affect how your code works - it
will do exactly the same thing either way.

So, the part of my original reply that may have been helpful was just that
you should remove the For Next loop from your code, so that it looks like:

Private Sub Workbook_SheetChange(ByVal ws As Object, ByVal Target As Range)
Dim sShName As String

If Right(ws.Name, 7) <> "Monthly" Then 'ignore these sheeets
Select Case ws.Name
Case "(Code Key)" 'Excluded sheets
Exit Sub
Case Else 'if password hasn't been entered yet, then ask for
password
If bPwrdEntrd = False Then
Call PasswordEntry
End If
End Select
End If

End Sub

This way you are only checking the worksheet that was changed.

With the For Next, you were checking all of the above for every sheet in the
workbook, until the loop got to "(Code Key)" and exited the sub. So, for
example, if you had a sheet named "Other" that was first in the workbook,
the For Next loop would check the above for it first, which would trigger
your Case Else and prompt for a password, no matter which sheet the user had
actually changed.

Does that make sense?

hth,

Doug

 

[davegb]

Hi Dave,

I did not mean to make too much of the "ws" v. "sh" thing. All I meant was
that if you create a WorkSheet_Change event in the VBE by using the dropdown
menus at the top to pick that event, it will start out with an empty
procedure that looks like:

Private Sub Workbook_SheetChange(ByVal Sh As Object, ByVal Target As Range)

I assumed (and I probably shouldn't) that you had started from this and for
some reason changed the "Sh" above - the "default" - to "ws". All I was
thinking is that it would be best to leave it at "Sh". This is certainly
not critical though, and we know that JE, who is far smarter at this, does
not agree.

You said exactly what I was thinking about the second part, regarding not
reusing "ws". Again, this does not actually affect how your code works - it
will do exactly the same thing either way.

So, the part of my original reply that may have been helpful was just that
you should remove the For Next loop from your code, so that it looks like:

Private Sub Workbook_SheetChange(ByVal ws As Object, ByVal Target As Range)
Dim sShName As String

If Right(ws.Name, 7) <> "Monthly" Then 'ignore these sheeets
Select Case ws.Name
Case "(Code Key)" 'Excluded sheets
Exit Sub
Case Else 'if password hasn't been entered yet, then ask for
password
If bPwrdEntrd = False Then
Call PasswordEntry
End If
End Select
End If

End Sub

This way you are only checking the worksheet that was changed.

With the For Next, you were checking all of the above for every sheet in the
workbook, until the loop got to "(Code Key)" and exited the sub. So, for
example, if you had a sheet named "Other" that was first in the workbook,
the For Next loop would check the above for it first, which would trigger
your Case Else and prompt for a password, no matter which sheet the user had
actually changed.

Does that make sense?

hth,

Doug

Thanks again, Doug.

Now I get it! It makes sense. I appreciate your taking the time to
elaborate. And please don't hesitate any time you see me making a
mistake that's "bad" programming practice, even though it might work.
I've found over the years that the experts are 99.99 percent right,
that they have good reasons for avoiding methods that often cause
trouble down the line. Someday, if I'm ever an "expert" in VBA, I can
decide then what good practices I should follow and when there's a good
reason to do otherwise. Right now, I'm just trying to get my job done
and avoid problems both in writing this code and for the end users in
using it. I think that will be facilitated by learning and using "good"
practices.

 

[Doug Glancy]

Dave,

I'm glad I was able to help

Doug

Thanks again, Doug.

Now I get it! It makes sense. I appreciate your taking the time to
elaborate. And please don't hesitate any time you see me making a
mistake that's "bad" programming practice, even though it might work.
I've found over the years that the experts are 99.99 percent right,
that they have good reasons for avoiding methods that often cause
trouble down the line. Someday, if I'm ever an "expert" in VBA, I can
decide then what good practices I should follow and when there's a good
reason to do otherwise. Right now, I'm just trying to get my job done
and avoid problems both in writing this code and for the end users in
using it. I think that will be facilitated by learning and using "good"
practices.