Home > Blockchain >  How can I write this Macro into few lines. I want to get rid of this multiple lines
How can I write this Macro into few lines. I want to get rid of this multiple lines

Time:05-05

  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 rngOut As Range
    Set rngOut = ThisWorkbook.Worksheets(1).Range("B21")

    rngOut.Value = vbNullString
    
    If Not (isOn(6) And isOn(7) And isOn(8) And isOn(9)) Then Exit Sub

    Dim a,b,c,d

    With ThisWorkbook
        a = isOn(1)
        b = isOn(5)
        c = .Worksheets(1).Range("B24").Value = "Speed"
        d = .Worksheets(1).Range("B26").Value = "Flow(from fill level)"
    
        If (a Or d) And (b Or c) Then rngOut.Value = "Flow drop"
    End With
End Sub
  • Related