Solved: Optimize Code

[gnod]

Hi,

I have 3 toggle buttons in a worksheet using this code

Option Explicit
Private Sub btnToggle_REV_AAGenCar_ActualTotalVol_Click()
Application.ScreenUpdating = False
With ActiveSheet
.Unprotect modRevMisc.strPassword
'Actual Total Volume - Jan to Dec Column
With .Range("REV_HideCols_AAGenCar_ActualTotalVol").EntireColumn
If btnToggle_REV_AAGenCar_ActualTotalVol = True Then
.Hidden = False
Else
.Hidden = True
End If
End With
.Protect modRevMisc.strPassword
End With
Application.ScreenUpdating = True
End Sub
Private Sub btnToggle_REV_AAGenCar_ProjTotalRev_Click()
Application.ScreenUpdating = False
With ActiveSheet
.Unprotect modRevMisc.strPassword
'Projected Total Revenue - Jan to Dec Column
With .Range("REV_HideCols_AAGenCar_ProjTotalRev").EntireColumn
If btnToggle_REV_AAGenCar_ProjTotalRev = True Then
.Hidden = False
Else
.Hidden = True
End If
End With
.Protect modRevMisc.strPassword
End With
Application.ScreenUpdating = True
End Sub
Private Sub btnToggle_REV_AAGenCar_ProjTotalVol_Click()
Application.ScreenUpdating = False
With ActiveSheet
.Unprotect modRevMisc.strPassword
'Projected Total Volume - Jan to Dec Column
With .Range("REV_HideCols_AAGenCar_ProjTotalVol").EntireColumn
If btnToggle_REV_AAGenCar_ProjTotalVol = True Then
.Hidden = False
Else
.Hidden = True
End If
End With
.Protect modRevMisc.strPassword
End With
Application.ScreenUpdating = True
End Sub


Can this 3 sub procedures combine into 1 procedure? :think:

Thanks

[XLGibbs]

Private Sub btnToggle_REV_AAGenCar_ProjTotalVol_Click()
.Unprotect modRevMisc.strPassword

ToggleHidden "REV_HideCols_AAGenCar_ProjTotalVol"

.Protect modRevMisc.strPassword

End Sub

Sub ToggleHidden(r As String)
Application.ScreenUpdating = False

If Range(r).EntireColumn.Hidden = False Then
Range(r).EntireColumn.Hidden = True
Else: Range(r).Hidden = False
End If

Application.ScreenUpdating = True
End Sub

I would have the hidden routine as its own routine, and pass the Range name or string through it...

tested with this:

Sub trf()
Dim strRangeName As String
strRangeName = "A1"

ToggleHidden strRangeName

End Sub

[JimmyTheHand]

Try this (not tested)
Private Sub btnToggle_REV_AAGenCar_ActualTotalVol_Click()
abcd Me.btnToggle_REV_AAGenCar_ActualTotalVol.Value
End Sub
Private Sub btnToggle_REV_AAGenCar_ProjTotalRev_Click()
abcd Me.btnToggle_REV_AAGenCar_ProjTotalRev.Value
End Sub
Private Sub btnToggle_REV_AAGenCar_ProjTotalVol_Click()
abcd Me.btnToggle_REV_AAGenCar_ProjTotalVol.Value
End Sub


Sub abcd(BoolPar As Boolean)
Application.ScreenUpdating = False
With ActiveSheet
.Unprotect modRevMisc.strPassword
'Projected Total Volume - Jan to Dec Column
.Range("REV_HideCols_AAGenCar_ProjTotalVol").EntireColumn .Hidden = Not BoolPar
.Protect modRevMisc.strPassword
End With
Application.ScreenUpdating = True
End Sub


EDIT:
Haven't noticed that range names are different. Update (still not tested):
Private Sub btnToggle_REV_AAGenCar_ActualTotalVol_Click()
abcd Me.btnToggle_REV_AAGenCar_ActualTotalVol.Value, "REV_AAGenCar_ActualTotalVol"
End Sub
Private Sub btnToggle_REV_AAGenCar_ProjTotalRev_Click()
abcd Me.btnToggle_REV_AAGenCar_ProjTotalRev.Value. "REV_AAGenCar_ProjTotalRev"
End Sub
Private Sub btnToggle_REV_AAGenCar_ProjTotalVol_Click()
abcd Me.btnToggle_REV_AAGenCar_ProjTotalVol.Value, "REV_AAGenCar_ProjTotalVol"
End Sub


Sub abcd(BoolPar As Boolean, StrPar as String)
Application.ScreenUpdating = False
With ActiveSheet
.Unprotect modRevMisc.strPassword
'Projected Total Volume - Jan to Dec Column
.Range(StrPar).EntireColumn .Hidden = Not BoolPar
.Protect modRevMisc.strPassword
End With
Application.ScreenUpdating = True
End Sub

[gnod]

XLGibbs,

thanks for your reply but your code didn't rely on the "Value" property of the Toggle button..


JimmyTheHand,

I test your code but it has an error in
.Range(StrPar).EntireColumn.Hidden = Not BoolPar

i already remove the space between the EntireColumn and Hidden in your code..

[gnod]

the error is "Application-defined or object-defined error" in
.Range(StrPar).EntireColumn.Hidden = Not BoolPar

[gnod]

i found a way to solve my problem.. you gave me an idea.. thanks for your reply.. here's the code i use

Private Sub btnToggle_REV_AAGenCar_ActualTotalVol_Click()
ToggleHidden Me.btnToggle_REV_AAGenCar_ActualTotalVol.Value, "REV_HideCols_AAGenCar_ActualTotalVol"
End Sub

'In standard Module
Sub ToggleHidden(ToggleStatus As Boolean, r As String)
Application.ScreenUpdating = False

With ActiveSheet
.Unprotect modRevMisc.strPassword
With .Range(r).EntireColumn
If ToggleStatus = True Then
.Hidden = False
Else
.Hidden = True
End If
End With
.Protect modRevMisc.strPassword
End With

Application.ScreenUpdating = True
End Sub

[Bob Phillips]

Sub ToggleHidden(ToggleStatus As Boolean, r As String)
Application.ScreenUpdating = False

With ActiveSheet
.Unprotect modRevMisc.strPassword
With .Range(r).EntireColumn
ToggleStatus.Hidden = Not ToggleStatus.Hidden
End With
.Protect modRevMisc.strPassword
End With

Application.ScreenUpdating = True
End Sub

[gnod]

ToggleStatus is a Boolean, it doesn't have a Hidden property

[moa]

maybe he meant something like this:

Private Sub btnToggle_REV_AAGenCar_ActualTotalVol_Click()
Application.ScreenUpdating = False
With ActiveSheet
.Unprotect modRevMisc.strPassword
With .Range("REV_HideCols_AAGenCar_ActualTotalVol").EntireColumn
.Hidden = Not .Hidden
End With
.Protect modRevMisc.strPassword
End With
Application.ScreenUpdating = True
End Sub



Edit: Didn't read the original post. But you catch my drift.

[moa]

Of course it probably (haven't tested it) does the same job as your code so why bother changing right?

[gnod]

i think :think: it is much better to pass the parameter to 1 procedure unlike before that i have 3 sub procedure which do the same thing..

[XLGibbs]

i think :think: it is much better to pass the parameter to 1 procedure unlike before that i have 3 sub procedure which do the same thing..

That is very wise. Often times, code re-usability is overlooked...I know I did when I frst started learning. But using 1 function to do a procedure that repeats based on different variables passed is much easier to maintain/change the actual procedure when you only have 1 of them.

If this is solved, feel free to mark it so.

Glad our combined efforts lead you to a workable solution.

[moa]

yeah... like I said I had not read your original post. Of course you should pass the range name to another sub so that you are not repeating yourself. xld (and later, myself) was just attempting to point out that you don't need the boolean variable so you can simplify the code a little.

Private Sub btnToggle_REV_AAGenCar_ActualTotalVol_Click()
ToggleHidden "REV_HideCols_AAGenCar_ActualTotalVol"
End Sub

'In standard Module
Sub ToggleHidden(r As String)
Application.ScreenUpdating = False
With ActiveSheet
.Unprotect modRevMisc.strPassword
.Range(r).EntireColumn.Hidden = Not .Hidden
.Protect modRevMisc.strPassword
End With
Application.ScreenUpdating = True
End Sub


...that was "my drift"

[XLGibbs]

yeah... like I said I had not read your original post. Of course you should pass the range name to another sub so that you are not repeating yourself. xld (and later, myself) was just attempting to point out that you don't need the boolean variable so you can simplify the code a little.

Private Sub btnToggle_REV_AAGenCar_ActualTotalVol_Click()
ToggleHidden "REV_HideCols_AAGenCar_ActualTotalVol"
End Sub

'In standard Module
Sub ToggleHidden(r As String)
Application.ScreenUpdating = False
With ActiveSheet
.Unprotect modRevMisc.strPassword
.Range(r).EntireColumn.Hidden = Not .Hidden
.Protect modRevMisc.strPassword
End With
Application.ScreenUpdating = True
End Sub

...that was "my drift"

And you are correct. I was merely demonstrating the thought process as well. Your way is more efficient for sure, but someone unfamiliar may not really follow that without the explanations now provided as to the boolean effect. I had originally written that way..or something like that..but wanted to demonstrate "how it worked".

As someone posted in the weekly class thread, sometimes we provide solutions, that while they work on a button click, the recipient won't understand what is happening and not necessarily learn from it.

I am always looking for someone to improve on code I provide though, so I never worry about someone posting a more efficient version...a lot of times I might not have thought of that way either.
:beerchug: