macro to group sheets then performing task

0

I have a macro to group all visible sheets and then hide/unhide certain columns (across all selected sheets) depending on the value of a cell.

When I run through the macro step-by-step, I can see that all the sheets are selected/grouped together when it's working through the IF statements, but when all is said and done, only the front sheet's columns have been adjusted according to the criteria and the rest stayed the way the way they were. Not sure what I'm missing?

Thanks.

Sub Col_Adj()
    Dim ws As Worksheet
    For Each ws In Sheets
        If ws.Visible Then ws.Select (False)
    Next

    If Range("ActPd") = "P00" Then
            Range("G:G,I:I,K:K,M:M,O:O,Q:Q,S:S,U:U,W:W,Y:Y,AA:AA,AC:AC").EntireColumn.Hidden = True            
            Range("H:H,J:J,L:L,N:N,P:P,R:R,T:T,V:V,X:X,Z:Z,AB:AB,AD:AD").EntireColumn.Hidden = False

    ElseIf Range("ActPd") = "P01 Oct" Then
            Range("H:H,I:I,K:K,M:M,O:O,Q:Q,S:S,U:U,W:W,Y:Y,AA:AA,AC:AC").EntireColumn.Hidden = True          
            Range("G:G,J:J,L:L,N:N,P:P,R:R,T:T,V:V,X:X,Z:Z,AB:AB,AD:AD").EntireColumn.Hidden = False
    End If

    Sheets("Summary").Select
End Sub
'
Answer
Discuss

Answers

0
Selected Answer

Hi Cathy,

The 3-D action the Select object is able to perform for the user isn't available to VBA, nor is it required. You can just loop through all sheets and do your adjustments on each of them. You would do better if you would avoid the Select object entirely unless you really want to make a selection for the user. For all manipulations use the Range object instead.

I'm impressed with the progress you are making. Therefore I took the time to re-write your code the way I would have programmed the same task and added comments. Ignore the following if you aren't interested.

Sub Col_Adj()
    ' 12 Dec 2018

    Const Selector As String = "P01 Oct"            ' change the month here (not in the code!)

    Dim Wb As Workbook                              ' I recommend to declare the workbook
    Dim Ws As Worksheet
    Dim FirstClm As Long, LastClm As Long
    Dim Act As Boolean
    Dim C As Long

    Set Wb = ActiveWorkbook
    Set Ws = Wb.Worksheets("Summary")               ' and declare the worksheet (while needed)
    Ws.Select                                       ' after this there is no more need for it
    Act = (Ws.Range("ActPd").Value = "P00")         ' not clear from your code where "ActPd" is located
    ' The following is probably not necesary (but your code has this condition):-
    ' Using StrComp makes the comparison case-insensitive in case of typos
    If (Not Act) And _
       (StrComp(Ws.Range("ActPd").Value, Selector, vbTextCompare) <> 0) Then _
       Exit Sub

    FirstClm = Columns("G").Column                  ' I would assign the value directly = 7
    LastClm = Columns("AD").Column                  ' I would assign the value directly = 30
    Application.ScreenUpdating = False

    For Each Ws In Sheets
        ' the Summary sheet isn't excluded from the following action
        If Ws.Visible Then
            ' obviously, this would be simpler if you could
            ' keep the columns together in contiguous blocks:-
            For C = FirstClm To LastClm
                Ws.Columns(C).EntireColumn.Hidden = Act
                Act = Not Act
            Next C
        End If
    Next


    Application.ScreenUpdating = True
End Sub

The main difference is that I pay much more attention to declarations, especially the objects. I think you will also like the way I designed the column selection (although, of course, they need no selection, just addressing). The simple logic is that you need every other column to be hidden, with a different state for the first column, depending upon the range "ActPd". So, instead of instructing which columns to hide, I concentrated the effort on setting the correct value for the first column and then created an alternating sequence to follow.

Observe that I turn off ScreenUpdating before the action starts and leave it off until the end. That speeds up the code significantly! Note that you can't Select anything on sheets that aren't active. The good news is that you don't need to select anything. It's all demonstrated in the attached workbook.

Discuss

Discussion

Wow, thanks for taking the time to do all that. Quite something to wrap my head around.

My macro isn't quite just alternating columns to be hidden. I only left in the first 2 If / Else If to keep the post simple. There's a few Else If after that.

Instead of using the For Each / Next to group all the worksheets, I've placed it around the If statement. But seems it's still not looping through all the visible sheets.

Sub Col_Adj()
 
    Dim Ws As Worksheet
 
    For Each Ws In Sheets
 
    If Ws.Visible Then
 
        If Range("ActPd") = "P00" Then
 
  ' various ElseIf statements to hide/unhide columns based on value of the range "ActPd"
        
        End If
        
    End If
    
    Next
 
    Sheets("Summary").Select
 
End Sub


The If statement works (though perhaps not the most efficient) but the macro is just not applying it to all the visible worksheets, only the first ("Summary") sheet.
Cathy (rep: 53) Dec 12, '18 at 9:39 am
Ah, never mind. I think I've got it working. 

I added "Ws.Select" right after If Ws.Visible Then

Thanks again.


Cathy (rep: 53) Dec 12, '18 at 10:10 am
Hi Cathy,
You definitely don't need Ws.Select. If the code starts behaving differently after you added Ws.Select the indication is that you omitted "Ws." in one or more of the commands that follow. Remember: without the leading "Ws." the ActiveSheet is specified by default instead of Ws.
You may like to read my today's post in this thread.
Variatus (rep: 2938) Dec 12, '18 at 8:35 pm
Add to Discussion

Answer the Question

You must create an account to use the forum. Create an Account or Login