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