1

hoping someone can help me out here.

I'm trying to put together some code that will hide rows based on the values of two cells.

My code is below:

 Sub hideSummaryDetailed()

 Application.ScreenUpdating = False
 Application.EnableEvents = False
 ActiveSheet.DisplayPageBreaks = False


 'Hide all the Rows based on the selection for Summary/Detailed data linked to cell A1
 If Cells(1, 1) = 0 Then
    Rows("23:336").Select Selection.EntireRow.Hidden = True

 ElseIf Cells(1, 1) = 1 And Cells(10, 5) = "All" Then
    Rows("23:43").Select Selection.EntireRow.Hidden = False
    Range("E11").Select
    Rows("44:336").Select Selection.EntireRow.Hidden = True

 ElseIf Cells(1, 1) = 2 And Cells(10, 5) = "All" Then
    Rows("23:126").Select Selection.EntireRow.Hidden = False
    Range("E11").Select
    Rows("127:336").Select Selection.EntireRow.Hidden = True

 ElseIf Cells(1, 1) = 1 And Cells(10, 5) = "Cardiff" Then
    Rows("128:148").Select Selection.EntireRow.Hidden = False
    Range("E11").Select
    Rows("23:127, 149:336").Select Selection.EntireRow.Hidden = True

 ElseIf Cells(1, 1) = 2 And Cells(10, 5) = "Cardiff" Then
    Rows("128:232").Select Selection.EntireRow.Hidden = False
    Range("E11").Select
    Rows("23:127, 233:336").Select Selection.EntireRow.Hidden = True

 ElseIf Cells(1, 1) = 1 And Cells(10, 5) = "Swansea" Then
    Rows("233:253").Select Selection.EntireRow.Hidden = False
    Range("E11").Select
    Rows("23:232, 254:336").Select Selection.EntireRow.Hidden = True

 ElseIf Cells(1, 1) = 2 And Cells(10, 5) = "Swansea" Then
    Rows("233:336").Select Selection.EntireRow.Hidden = False
    Range("E11").Select
    Rows("23:232").Select Selection.EntireRow.Hidden = True

 ElseIf Cells(1, 1) = 1 And Cells(10, 5) = "Both" Then
    Rows("128:148, 233:253").Select Selection.EntireRow.Hidden = False
    Range("E11").Select
    Rows("23:127, 149:232, 254:336").Select Selection.EntireRow.Hidden = True

 ElseIf Cells(1, 1) = 2 And Cells(10, 5) = "Both" Then
    Rows("128:336").Select Selection.EntireRow.Hidden = False
    Range("E11").Select
    Rows("23:127").Select Selection.EntireRow.Hidden = True

 End If

 Application.ScreenUpdating = True
 Application.EnableEvents = True

 Range("E11").Select

 End Sub

However when I run it I keep getting an error saying Wrong Number of Arguments or Invalid Property Assignments, when I click ok it doesn't highlight any specific part of the code to point me in the direction.

I am pretty new to VBA and have searched a few forums for advice on this error but nothing I have tried has worked or I have not understood it.

Any help would be very much appreciated.

Thanks

K15
  • 13
  • 5

2 Answers2

5

This is not valid VBA code:

Rows("23:43").Select Selection.EntireRow.Hidden = False

Remember, a single line in VBA is a single command. The above line is two separate commands. This is valid VBA code:

Rows("23:43").Select 
Selection.EntireRow.Hidden = False

However now we deal with the inefficiency of the code itself. You are using ActiveSheet implicitly, and you are relying on Select. For avoiding Select and Activate I would recommend starting here: How to avoid using Select in Excel VBA .

How can we refactor this code? Simple:

' Near the beginning of the module....
Dim Target as Worksheet

' Ideally, explicitly set this to the correct worksheet. 
' For now, this will still use the Activesheet, it will just
' do it more explicitly and reliably since it won't potentially change as the code is running.
Set Target = ActiveSheet


' Later in the code....

' Notice how clean this is. We rely on Target instead of an implicit ActiveSheet, and we
' don't have to rely on the `EntireRow` property of `Selection` since we are explicitly accessing
' the rows.

Target.Rows("23:24").Hidden = False

This not only solves the error at hand, but it also helps you avoid many of the common errors that will cause countless hours of frustration and maintenance later.

Brandon Barney
  • 2,382
  • 1
  • 9
  • 18
  • As far as I'm aware, neither is Cells without a Worksheet, aka the syntax needs to be Worksheets("worksheet_name").Cells(a, b) rather than just Cells(a,b) – acousticismX Sep 13 '17 at 15:53
  • 1
    @acousticismX That is correct. Pretty much any time you are missing a higher-level reference on the left hand side you will want to see if it is properly qualified. Preferably, everything should be qualified up to the Workbook level. – Brandon Barney Sep 13 '17 at 15:54
  • thanks for confirming. it's been a while since I've done VBA. – acousticismX Sep 13 '17 at 15:56
  • I'm amazed that the single line of code that should be two compiles - doesn't run, but it compiles. Being pedantic (what else would you expect from a coder) - you could have it as a single line if the commands are separated by a colon: `Rows("23:43").Select: Selection.EntireRow.Hidden = False`. But that makes it hard to read and 'tis silly. – Darren Bartrup-Cook Sep 13 '17 at 16:22
  • 1
    @DarrenBartrup-Cook Agreed. My initial reaction is `That shouldn't compiile` but sure enough it did. I thought of introducing the `:` line continuation token, but it causes far more grief than it is worth in the long run. – Brandon Barney Sep 13 '17 at 16:25
  • Thanks Brandon, great help! – K15 Sep 18 '17 at 07:43
1

Below is the solution for the problem mentioned above.

I have re-factored code.

In below code replace SheetName "Sample" with the name of your sheet in workbook.

 Sub hideSummaryDetailed()

 Application.ScreenUpdating = False
 Application.EnableEvents = False
 ActiveSheet.DisplayPageBreaks = False

 Dim ws As Worksheet
 Set ws = ThisWorkbook.Worksheets("Sample")

 With ws

     'Hide all the Rows based on the selection for Summary/Detailed data linked to cell A1
     If .Cells(1, 1) = 0 Then
        .Range("A23:A336").EntireRow.Hidden = False

     ElseIf .Cells(1, 1) = 1 And .Cells(10, 5) = "All" Then
        .Range("A23:A336").EntireRow.Hidden = False
        .Range("A44:A336").EntireRow.Hidden = True

     ElseIf .Cells(1, 1) = 2 And .Cells(10, 5) = "All" Then
        .Range("A23:A336").EntireRow.Hidden = False
        .Range("A127:3A36").EntireRow.Hidden = True

     ElseIf .Cells(1, 1) = 1 And .Cells(10, 5) = "Cardiff" Then
        .Range("A23:A336").EntireRow.Hidden = False
        .Range("A23:A127, A149:A336").EntireRow.Hidden = True

     ElseIf .Cells(1, 1) = 2 And .Cells(10, 5) = "Cardiff" Then
        .Range("A23:A336").EntireRow.Hidden = False
        .Range("A23:A127, A233:A336").EntireRow.Hidden = True

     ElseIf .Cells(1, 1) = 1 And .Cells(10, 5) = "Swansea" Then
        .Range("A23:A336").EntireRow.Hidden = False
        .Range("A23:A232, A254:A336").EntireRow.Hidden = True

     ElseIf .Cells(1, 1) = 2 And .Cells(10, 5) = "Swansea" Then
        .Range("A23:A336").EntireRow.Hidden = False
        .Range("A23:A232").EntireRow.Hidden = True

     ElseIf .Cells(1, 1) = 1 And .Cells(10, 5) = "Both" Then
        .Range("A23:A336").EntireRow.Hidden = False
        .Range("A23:A127, A149:A232, A254:A336").EntireRow.Hidden = True

     ElseIf .Cells(1, 1) = 2 And .Cells(10, 5) = "Both" Then
        .Range("A23:A336").EntireRow.Hidden = False
        .Range("A23:A127").EntireRow.Hidden = True

     End If

 End With

 ws.Range("E11").Select

 Application.ScreenUpdating = True
 Application.EnableEvents = True

 End Sub
Abhinav Asthana
  • 101
  • 2