How can I write this Macro into few lines. I want to get rid of this multiple lines-CodePudding

  Sub Flow_Drop() 

   If  ThisWorkbook.Worksheets(2).CHECKBOXES("Check Box 1").Value = xlOn 
       And ThisWorkbook.Worksheets(2).CHECKBOXES("Check Box 5").Value = xlOn 
       And ThisWorkbook.Worksheets(2).CHECKBOXES("Check Box 6").Value = xlOn 
       And ThisWorkbook.Worksheets(2).CHECKBOXES("Check Box 7").Value = xlOn 
       And ThisWorkbook.Worksheets(2).CHECKBOXES("Check Box 8").Value = xlOn 
       And ThisWorkbook.Worksheets(2).CHECKBOXES("Check Box 9").Value = xlOn Then
       ThisWorkbook.Worksheets(1).Range("B21").Value = "Flow drop"

       Else
        If ThisWorkbook.Worksheets(2).CHECKBOXES("Check Box 1").Value = xlOn 
        And ThisWorkbook.Worksheets(1).Range("B24").Value = "Speed" 
        And ThisWorkbook.Worksheets(2).CHECKBOXES("Check Box 6").Value = xlOn 
        And ThisWorkbook.Worksheets(2).CHECKBOXES("Check Box 7").Value = xlOn
        And ThisWorkbook.Worksheets(2).CHECKBOXES("Check Box 8").Value = xlOn 
        And ThisWorkbook.Worksheets(2).CHECKBOXES("Check Box 9").Value = xlOn Then
        ThisWorkbook.Worksheets(1).Range("B21").Value = "Flow drop"

         Else
           If ThisWorkbook.Worksheets(1).Range("B26").Value = "Flow(from fill level)" 
           And ThisWorkbook.Worksheets(2).CHECKBOXES("Check Box 5").Value = xlOn 
           And ThisWorkbook.Worksheets(2).CHECKBOXES("Check Box 6").Value = xlOn 
           And ThisWorkbook.Worksheets(2).CHECKBOXES("Check Box 7").Value = xlOn 
           And ThisWorkbook.Worksheets(2).CHECKBOXES("Check Box 8").Value = xlOn 
           And ThisWorkbook.Worksheets(2).CHECKBOXES("Check Box 9").Value = xlOn Then
           ThisWorkbook.Worksheets(1).Range("B21").Value = "Flow drop"

           Else
           If ThisWorkbook.Worksheets(1).Range("B26").Value = "Flow(from fill level)" 
           And ThisWorkbook.Worksheets(1).Range("B24").Value = "Speed" 
           And ThisWorkbook.Worksheets(2).CHECKBOXES("Check Box 6").Value = xlOn 
           And ThisWorkbook.Worksheets(2).CHECKBOXES("Check Box 7").Value = xlOn 
           And ThisWorkbook.Worksheets(2).CHECKBOXES("Check Box 8").Value = xlOn 
           And ThisWorkbook.Worksheets(2).CHECKBOXES("Check Box 9").Value = xlOn Then
           ThisWorkbook.Worksheets(1).Range("B21").Value = "Flow drop"

              Else
               ThisWorkbook.Worksheets(1).Range("B21").Value = vbNullString

              End If
          End If
         End If
      End If
    End Sub

This code is generated within two sheet sheet 2 contain multiple check boxes and when the check boxes are on the the variables in sheet 1 appears automatically. I want to make this long code in to few lines of code

CodePudding user response：

There are multiple areas of improvement. Saving Worksheets(1) and Worksheets(2) as variables means you can use a shorter name to refer to them in your code.

    Dim WS1 As Worksheet
    Set WS1 = ThisWorkbook.Worksheets(1)

    Dim WS2 As Worksheet
    Set WS2 = ThisWorkbook.Worksheets(2)

Every If statement has Check Box 6,7,8,9 so doing that ahead of the If Statements and saving the result as a variable would significantly cut down on repeated code:

    Dim CB6789 As Boolean
    CB6789 = WS2.CheckBoxes("Check Box 6").Value = xlOn _
             And WS2.CheckBoxes("Check Box 7").Value = xlOn _
             And WS2.CheckBoxes("Check Box 8").Value = xlOn _
             And WS2.CheckBoxes("Check Box 9").Value = xlOn

Now instead of including those 4 lines on everything, you can just check CB6789.

Then the next thing I notice is that you have Else: If instead of ElseIf. You're nesting the If statements instead expanding the first one. Compare the following:

'''''''''''''''''''
If A Then
    
Else
    If B Then
        
    Else
        If C Then
            
        Else
            If D Then
            
            End If
        End If
    End If
End If
'''''''''''''''''''''
If A Then

ElseIf B Then

ElseIf C Then

ElseIf D Then

End If
'''''''''''''''''''''

Personally, I find the second format much easier to follow, much neater to look at and much easier to edit or expand.

Finally, Most of your results are actually WS1.Range("B21").Value = "Flow drop". This action happens in most cases of your If Statement tree. Those branches can be combined. For example:

'''''''''''''''''''
    If A Then
        MsgBox 1
    ElseIf B Then
        MsgBox 1
    End If
'''''''''''''''''''
    If A Or B Then
        MsgBox 1
    End If
'''''''''''''''''''

Putting all those pieces of advice together:

Sub Flow_Drop()
    Dim WS1 As Worksheet
    Set WS1 = ThisWorkbook.Worksheets(1)

    Dim WS2 As Worksheet
    Set WS2 = ThisWorkbook.Worksheets(2)
    
    Dim CB6789 As Boolean
    CB6789 = WS2.CheckBoxes("Check Box 6").Value = xlOn _
             And WS2.CheckBoxes("Check Box 7").Value = xlOn _
             And WS2.CheckBoxes("Check Box 8").Value = xlOn _
             And WS2.CheckBoxes("Check Box 9").Value = xlOn
    
    Dim CB1 As Boolean
    CB1 = WS2.CheckBoxes("Check Box 1").Value = xlOn

    Dim CB5 As Boolean
    CB5 = WS2.CheckBoxes("Check Box 5").Value = xlOn
    
    Dim IsSpeed As Boolean
    IsSpeed = WS1.Range("B24").Value = "Speed"
    
    Dim IsFlow As Boolean
    IsFlow = WS1.Range("B26").Value = "Flow(from fill level)"

    If CB6789 _
    And (CB1 Or IsFlow) _
    And (CB5 Or IsSpeed) _
    Then
        WS1.Range("B21").Value = "Flow drop"
        
    Else
        WS1.Range("B21").Value = vbNullString
    End If
End Sub

Alternatively, since the question was about reducing the number of lines:

Sub Flow_Drop()
    ThisWorkbook.Worksheets(1).Range("B21").Value = IIf(ThisWorkbook.Worksheets(2).CheckBoxes("Check Box 6").Value = xlOn And ThisWorkbook.Worksheets(2).CheckBoxes("Check Box 7").Value = xlOn And ThisWorkbook.Worksheets(2).CheckBoxes("Check Box 8").Value = xlOn And ThisWorkbook.Worksheets(2).CheckBoxes("Check Box 9").Value = xlOn And (ThisWorkbook.Worksheets(2).CheckBoxes("Check Box 1").Value = xlOn Or ThisWorkbook.Worksheets(1).Range("B26").Value = "Flow(from fill level)") And (ThisWorkbook.Worksheets(2).CheckBoxes("Check Box 5").Value = xlOn Or ThisWorkbook.Worksheets(1).Range("B24").Value = "Speed"), "Flow drop", vbNullString)
End Sub

CodePudding user response：

You could maybe tidy it up a bit like this:

Sub Flow_Drop()
    Dim s1 As Worksheet, s2 As Worksheet
    Dim check1 As Boolean, check5 As Boolean, check6789 As Boolean, flow As Boolean, speed As Boolean
    
    Set s1 = ThisWorkbook.Worksheets(1)
    Set s2 = ThisWorkbook.Worksheets(2)
    
    check1 = s2.CheckBoxes("Check Box 1").Value = xlOn
    check5 = s2.CheckBoxes("Check Box 5").Value = xlOn
    check6789 = s2.CheckBoxes("Check Box 6").Value = xlOn _
           And s2.CheckBoxes("Check Box 7").Value = xlOn _
           And s2.CheckBoxes("Check Box 8").Value = xlOn _
           And s2.CheckBoxes("Check Box 9").Value = xlOn
    flow = s1.Range("B26").Value = "Flow(from fill level)"
    speed = s1.Range("B24").Value = "Speed"

    If check6789 And _
        ((check1 And check5) Or _
        (check1 And speed) Or _
        (flow And check5) Or _
        (flow And speed)) Then
            s1.Range("B21").Value = "Flow drop"
    Else
        s1.Range("B21").Value = vbNullString
    End If
    
End Sub

CodePudding user response：

Shortened version

Sub Flow_Drop()
Dim bolFD as Boolean 'boolean representing whether B21 should be 'Flow Drop' or not
Dim sht1 as Worksheet, sht2 as Worksheet

bolFD = True
With ThisWorkbook
   Set sht1 = .Worksheets(1): Set sht2 = .Worksheets(2)
   For a = 6 To 9 'the checkboxes first
       bolFD = sht2.CHECKBOXES("Check Box " & a).Value = xlOn
       If Not bolfd Then Exit For
   Next
   
   If Not _
        (sht1.CHECKBOXES("Check Box 1").Value = xlOn Or sht1.Range("B26").Value = "Flow(from fill level)") _
    Or Not _
        (sht2.CHECKBOXES("Check Box 5").Value = xlOn Or sht1.Range("B24").Value = "Speed" ) _
    Then bolFD = False

    If bolFD Then 
        sht1.Range("B21").Value = "Flow drop"
    Else
        sht1.Range("B21").Value = vbNullString
    End If
End With

End Sub

CodePudding user response：

First off, there are many repeated checks on boxes 6,7,8,9. All these have to be xlOn, so we can short-circuit the routine here and exit if not true.

Then let's say:

a = Checkbox(1) is On

b = Checkbox(5) is On

c = Range("B24") is "Speed"

d = Range("B26") is "Flow(fill from level)"

In Boolean algebra notation, the output range will be set to "Flow drop", if

(a * b) (a * c) (d * b) (d * c) is true

= a * (b c) d * (b c)

= (a d) * (b c)

Putting in a function helper for checkboxes yields this:

Private Function isOn(nBox As Integer) As Boolean
    isOn = ThisWorkbook.Worksheets(2).CheckBoxes("Check Box " & nBox).Value = xlOn
End Function

Sub flow_drop()
    Dim ws as Worksheet, rngOut As Range
    Set ws = ThisWorkbook.Worksheets(1)
    Set rngOut = ws.Range("B21")

    rngOut.Value = vbNullString

    If Not (isOn(6) And isOn(7) And isOn(8) And isOn(9)) Then Exit Sub
   
    Dim c,d
    c = ws.Range("B24").Value = "Speed"
    d = ws.Range("B26").Value = "Flow(from fill level)"
    
    If (isOn(1) Or d) And (isOn(5) Or c) Then rngOut.Value = "Flow drop"
End Sub