VB.Net Function Optimization (SSRS Report Code)
I came across the following code recently and would like to optimize it:
Public Shared Function ComputeLabel(ByVal Action As Integer, ByVal Flag1 As Boolean, ByVal Flag2 As Boolean) As String
Dim Prefix As String = ""
If Flag2 Then
Prefix = "Conditional "
ElseIf Flag1 Then
Prefix = "Held "
End If
Select Case Action
Case 0
Return ""
Case 1
Return Prefix & "Cancelled"
Case 2
Return Prefix & "Discontinued"
Case 3
Return Prefix & "Suspended"
Case 4
Return Prefix & "Unsuspended"
Case 6
Return Prefix & "Collected"
Case 7
Return Prefix & "Released from Hold"
Case 8
Return Prefix & "Modified"
Case 9
Return Prefix & "Discontinued for the Future"
Case 10
Return Prefix & "Verified"
Case 11
Return Prefix & "Modified And Verified"
Case 12
Return "Hold " & Prefix & "Cancelled"
Case Else
Return ""
End Select
End Function
Note that Action 0 is the most common case.
Okay, I've already cleaned up this function a bit--it was using a variable and returning it at the end, and using Return seems better. But additionally, I think it would be better code to build an array at the beginning of the report execution, and then just access array elements each time this function i开发者_C百科s called, instead of using a Select statement. But case 12 is making things more complicated (as you can see, it adds the prefix in the middle instead of at the beginning.)
What do you think would be the best way:
One time building a 39-element array for the three cases:
Private Shared OrderActions() As String = {"", "Cancelled", ...}Then in the function accessing it like so:
If Action < 0 OrElse Action >= 13 Then Return "" Return OrderActions(Action - Flag2 * 13 - (Flag1 AndAlso Not Flag2) * 26)Using a 13-element array with a Replace (something like
Return Replace(LabelList(Action), "{Prefix}", Prefix)?)Using a 12-element array with a special case for Action 12.
Something else I haven't thought of.
?
Update 1: my formatting was off so the options might have been unclear. It should be more readable now.
Update 2: I see what you mean that from a performance perspective, fully expanding all the cases and using simple variable assignment is probably fastest. So... let's say top speed is not the priority, but overall elegance is (a combination of clean code and speed). Any chance people could give their take on that, too? I'll vote everyone up who gives reasonable help for all aspects of the question.
Update 3: One additional consideration I was ignoring is that some non-experienced programmers are going to be maintaining this long-term, so it does need to be easy to understand. I guess my examples of trying to shorten the code really aren't good from this perspective.
Update 4: TESTING IS KING OVER ALL!!! Once I was inspired to do some speed tests, I got some interesting results. See my answer below.
Well if this is a bottleneck and speed is a priority then my first suggestion is to eliminate the string concatenation by duplicating much of your logic. This will make the function bigger and less readable, but maybe that's worth it for the speed...
Public Shared Function ComputeLabel(ByVal Action As Integer, ByVal Flag1 As Boolean, ByVal Flag2 As Boolean) As String
Select Case nHVCOrderAction
Case 0
Return ""
Case 1
If Flag2 Then
Return "Conditional Cancelled"
ElseIf Flag1 Then
Return "Held Cancelled"
Else
Return "Cancelled"
End If
Case 2
If Flag2 Then
Return "Conditional Discontinued"
ElseIf Flag1 Then
Return "Held Discontinued"
Else
Return "Discontinued"
End If
' And so on...
End Select
End Function
Relatively speaking the string concatenation in your code is probably going to be slower than the integer and boolean comparison operations you have here. So with enough repetition you should see a noticeable improvement in speed with this method.
UPDATE:
I wrote a quick & dirty VB.NET console app to test your original code vs mine. Here's the loop I ran for each (I didn't write every possible combination but you get the idea):
Dim sw As New Stopwatch()
sw.Start()
For i As Integer = 0 To 10000000
str = ComputeLabel(0, True, False)
str = ComputeLabel(1, False, False)
str = ComputeLabel(0, False, False)
str = ComputeLabel(2, False, False)
str = ComputeLabel(1, False, True)
str = ComputeLabel(2, True, True)
str = ComputeLabel(4, False, True)
str = ComputeLabel(7, True, True)
str = ComputeLabel(12, False, True)
Next
sw.Stop()
Console.WriteLine(sw.ElapsedMilliseconds & " ms")
And here are the times:
Old method: 6189 ms
New method: 1374 ms
So there's a 5X speed improvement by removing the string concatenation and expanding the conditions for each case. Of course, as you may have noticed that loop runs 10 million times... that's a lot.
UPDATE:
I wrote another benchmark app to mimic your own exactly:
Module Module1
Sub Main()
Dim str As String = ""
Dim sw As New Stopwatch()
' Test 1
sw.Start()
For i As Integer = 0 To 100000
For j As Integer = -1 To 13
str = ComputeLabel_X(j, False, False)
str = ComputeLabel_X(j, True, False)
str = ComputeLabel_X(j, False, True)
Next
Next
sw.Stop()
Console.WriteLine("Old method: " & sw.ElapsedMilliseconds & " ms")
sw.Reset()
' Test 2
sw.Start()
For i As Integer = 0 To 100000
For j As Integer = -1 To 13
str = ComputeLabel_Y(j, False, False)
str = ComputeLabel_Y(j, True, False)
str = ComputeLabel_Y(j, False, True)
Next
Next
sw.Stop()
Console.WriteLine("New method: " & sw.ElapsedMilliseconds & " ms")
sw.Reset()
End Sub
Public Function ComputeLabel_X(ByVal Action As Integer, ByVal Held As Boolean, ByVal Conditional As Boolean) As String
Dim Prefix As String = ""
If Conditional Then
Prefix = "Conditional "
ElseIf Held Then
Prefix = "Held "
End If
Select Case Action
Case 0
Return ""
Case 1
Return Prefix & "Cancelled"
Case 2
Return Prefix & "Discontinued"
Case 3
Return Prefix & "Suspended"
Case 4
Return Prefix & "Unsuspended"
Case 6
Return Prefix & "Collected"
Case 7
Return Prefix & "Released from Hold"
Case 8
Return Prefix & "Modified"
Case 9
Return Prefix & "Discontinued for the Future"
Case 10
Return Prefix & "Verified"
Case 11
Return Prefix & "Modified And Verified"
Case 12
Return "Hold " & Prefix & "Cancelled"
Case Else
Return ""
End Select
End Function
Public Function ComputeLabel_Y(ByVal Action As Integer, ByVal Held As Boolean, ByVal Conditional As Boolean) As String
Select Case Action
Case 0
Return ""
Case 1
If Conditional Then
Return "Conditional Cancelled"
ElseIf Held Then
Return "Held Cancelled"
Else
Return "Cancelled"
End If
Case 2
If Conditional Then
Return "Conditional Discontinued"
ElseIf Held Then
Return "Held Discontinued"
Else
Return "Discontinued"
End If
Case 3
If Conditional Then
Return "Conditional Suspended"
ElseIf Held Then
Return "Held Suspended"
Else
Return "Suspended"
End If
Case 4
If Conditional Then
Return "Conditional Unsuspended"
ElseIf Held Then
Return "Held Unsuspended"
Else
Return "Unsuspended"
End If
Case 6
If Conditional Then
Return "Conditional Collected"
ElseIf Held Then
Return "Held Collected"
Else
Return "Collected"
End If
Case 7
If Conditional Then
Return "Conditional Released from Hold"
ElseIf Held Then
Return "Held Released from Hold"
Else
Return "Released from Hold"
End If
Case 8
If Conditional Then
Return "Conditional Modified"
ElseIf Held Then
Return "Held Modified"
Else
Return "Modified"
End If
Case 9
If Conditional Then
Return "Conditional Discontinued for the Future"
ElseIf Held Then
Return "Held Discontinued for the Future"
Else
Return "Discontinued for the Future"
End If
Case 10
If Conditional Then
Return "Conditional Verified"
ElseIf Held Then
Return "Held Verified"
Else
Return "Verified"
End If
Case 11
If Conditional Then
Return "Conditional Modified And Verified"
ElseIf Held Then
Return "Held Modified And Verified"
Else
Return "Modified And Verified"
End If
Case 12
If Conditional Then
Return "Hold Conditional Cancelled"
ElseIf Held Then
Return "Hold Held Cancelled"
Else
Return "Hold Cancelled"
End If
Case Else
Return ""
End Select
End Function
End Module
And my results are again consistently much faster with my code:
Old method: 169 ms
New method: 30 ms
I am of course running without debugging (Ctrl F5). And now I'm on an 3.0 GHz AMD quad core.
actually, if you put them into arrays, it brings the execution time from 6589ms on my machine to 1174ms using the following method:
Below is from a console app, but you get the general idea. The arrays are loaded once, then accessed as many times as you want, in this case I was using the for loop that Steve Wortham posted as a test.
Dim _ConditionalLabels(12) As String
Dim _HeldLabels(12) As String
Sub Main()
Dim sw As New Stopwatch()
sw.Start()
_ConditionalLabels(0) = ""
_ConditionalLabels(1) = "Conditional Cancelled"
_ConditionalLabels(2) = "Conditional Discontinued"
_ConditionalLabels(3) = "Conditional Suspended"
_ConditionalLabels(4) = "Conditional Unsuspended"
_ConditionalLabels(6) = "Conditional Collected"
_ConditionalLabels(7) = "Conditional Released from Hold"
_ConditionalLabels(8) = "Conditional Modified"
_ConditionalLabels(9) = "Conditional Discontinued for the Future"
_ConditionalLabels(10) = "Conditional Verified"
_ConditionalLabels(11) = "Conditional Modified And Verified"
_ConditionalLabels(12) = "Hold Conditional Cancelled"
_HeldLabels(0) = ""
_HeldLabels(1) = "Held Cancelled"
_HeldLabels(2) = "Held Discontinued"
_HeldLabels(3) = "Held Suspended"
_HeldLabels(4) = "Held Unsuspended"
_HeldLabels(6) = "Held Collected"
_HeldLabels(7) = "Held Released from Hold"
_HeldLabels(8) = "Held Modified"
_HeldLabels(9) = "Held Discontinued for the Future"
_HeldLabels(10) = "Held Verified"
_HeldLabels(11) = "Held Modified And Verified"
_HeldLabels(12) = "Hold Held Cancelled"
Dim str As String = ""
For i As Integer = 0 To 10000000
str = ComputeLabel(0, True, False)
str = ComputeLabel(1, False, False)
str = ComputeLabel(0, False, False)
str = ComputeLabel(2, False, False)
str = ComputeLabel(1, False, True)
str = ComputeLabel(2, True, True)
str = ComputeLabel(4, False, True)
str = ComputeLabel(7, True, True)
str = ComputeLabel(12, False, True)
Next
sw.Stop()
Console.WriteLine(sw.ElapsedMilliseconds & " ms")
Console.Read()
End Sub
Public Function ComputeLabel(ByVal Action As Integer, ByVal Held As Boolean, ByVal Conditional As Boolean) As String
If Conditional Then
Return _ConditionalLabels(Action)
ElseIf Held Then
Return _HeldLabels(Action)
End If
End Function
You can't optimize it, it is already very efficient. You can make it more readable though, Flag1 and Flag2 definitely ought to be renamed to Held and Conditional.
Here are the results of my own speed testing (finally).
In my tests I used the following code. I used 1 million iterations to keep wait times down. -1 to 13 is to give some out of bounds work. When array setup is required, it was included in the total time, but only done once. Each function got its own calling procedure with the name (X) hardcoded:
Dim str As String = ""
For i As Integer = 0 To 1000000
For j As Integer = -1 To 13
str = ComputeLabel_X(j, False, False)
str = ComputeLabel_X(j, True, False)
str = ComputeLabel_X(j, False, True)
Next
Next
I also got different results when running the code with F5 (break into code on error) and Ctrl-F5 (run outside of debugger). I guess the second one has more validity for the SSRS code environment as nothing would be attaching to the process to debug it.
All results are compile without debug (Ctrl-F5).
- Russ: 613 ms (bounds checking added)
- Erik: 614 ms (same as Russ but arrays populated as array literal {} instead of individual statements)
- Erik2: 526 ms (0 to 12, with an extra 0 to 2, no bounds checking)
- Steve: 660 ms
- Steve2: 873 ms (remove ElseIf for individual If statements)
- New: 2977 ms (The first idea from my previous answer)
- Initial Post: 3915 ms * - revised to be correct (was 10 times too small)
- Original: 3947 ms (the version you've never seen, the one I optimized to post here)
- Choose: 11068 ms
- BigArray: 12565 ms (calculating the index into a big array with math)
Even though execution times could fluctuate by as much as 100 ms for the higher values, the rankings tended to stay consistent, except for the Russ and Erik versions which kept swapping.
Takeaways:
Building the array once is insignificant. Doing it as individual statements or as an array literal {} is identical.
It cost 20% more to do bounds checking for the array method.
Expanding the entire thing intuitively seems like it should be fastest, but it isn't. I don't know why. Perhaps it has something to do with processor cache line size and preloading tradeoffs, or something like that.
The only real changes I made from the original function to the function I posted in my question were: 1) Return from each case statement instead of assign the string to a variable and return that at the end (plus removing the variable), 2) swap the independent If Flag statements order and change the second If to an ElseIf. The 1% improvement was insignificant
It seems like I should be able to generalize something from the fact that my version listed as "New" (the first query in the other answer I posted) did so badly. Is it the longer strings? Is it that Returning is equally fast no matter where in the procedure it's done, but falling out of the Case statement to execute more instructions is slow?Russ's array version is fastest. An array lookup is faster than a case statement with string concatenation.
At this point I am not sure how to explain why the New version is faster.
The Choose function is super, super slow.
Because of these tests I have to award the answer for this question to nobugz who claimed that I couldn't optimize the given code. So far, he's been right!
Update: I am really sorry about the mistake I made that left off a zero on the number of iterations testing the Initial Post version. Things have been corrected now.
Addendum: The (corrected) testing code:
Module Module1
Dim _Labels(12) As String
Dim _ConditionalLabels(12) As String
Dim _HeldLabels(12) As String
Dim Labels() As String
Dim ConditionalLabels() As String
Dim HeldLabels() As String
Dim OrderLabelsBigArray(38) As String
Sub Main()
Dim sw As New Stopwatch()
sw.Start()
ComputeLabelsFirstPosted()
sw.Stop()
Console.WriteLine("FirstPosted " & sw.ElapsedMilliseconds & " ms")
sw.Reset()
sw.Start()
ComputeLabelsRuss()
sw.Stop()
Console.WriteLine("Russ " & sw.ElapsedMilliseconds & " ms")
sw.Reset()
sw.Start()
ComputeLabelsErik()
sw.Stop()
Console.WriteLine("Erik " & sw.ElapsedMilliseconds & " ms")
sw.Reset()
sw.Start()
ComputeLabelsErik2()
sw.Stop()
Console.WriteLine("Erik2 " & sw.ElapsedMilliseconds & " ms")
sw.Reset()
sw.Start()
ComputeLabelsBigArray()
sw.Stop()
Console.WriteLine("BigArray " & sw.ElapsedMilliseconds & " ms")
sw.Reset()
sw.Start()
ComputeLabelsSteve()
sw.Stop()
Console.WriteLine("Steve " & sw.ElapsedMilliseconds & " ms")
sw.Reset()
sw.Start()
ComputeLabelsSteve2()
sw.Stop()
Console.WriteLine("Steve2 " & sw.ElapsedMilliseconds & " ms")
sw.Reset()
sw.Start()
ComputeLabelsNew()
sw.Stop()
Console.WriteLine("New " & sw.ElapsedMilliseconds & " ms")
sw.Reset()
sw.Start()
ComputeLabelsChoose()
sw.Stop()
Console.WriteLine("Choose " & sw.ElapsedMilliseconds & " ms")
sw.Reset()
sw.Start()
ComputeLabelsOriginal()
sw.Stop()
Console.WriteLine("Original " & sw.ElapsedMilliseconds & " ms")
Console.Read()
End Sub
Public Sub ComputeLabelsFirstPosted()
Dim str As String = ""
For i As Integer = 0 To 1000000
For j As Integer = -1 To 13
str = ComputeLabelFirstPosted(j, False, False)
str = ComputeLabelFirstPosted(j, True, False)
str = ComputeLabelFirstPosted(j, False, True)
Next
Next
End Sub
Public Function ComputeLabelFirstPosted(ByVal Action As Integer, ByVal IsHeld As Boolean, ByVal IsConditional As Boolean) As String
Dim Prefix As String = ""
If IsConditional Then
Prefix = "Conditional "
ElseIf IsHeld Then
Prefix = "Held "
End If
Select Case Action
Case 0
Return ""
Case 1
Return Prefix & "Cancelled"
Case 2
Return Prefix & "Discontinued"
Case 3
Return Prefix & "Suspended"
Case 4
Return Prefix & "Unsuspended"
Case 6
Return Prefix & "Collected"
Case 7
Return Prefix & "Released from Hold"
Case 8
Return Prefix & "Modified"
Case 9
Return Prefix & "Discontinued for the Future"
Case 10
Return Prefix & "Verified"
Case 11
Return Prefix & "Modified And Verified"
Case 12
Return "Hold " & Prefix & "Cancelled"
Case Else
Return ""
End Select
End Function
Sub ComputeLabelsRuss()
_Labels(0) = ""
_Labels(1) = "Cancelled"
_Labels(2) = "Discontinued"
_Labels(3) = "Suspended"
_Labels(4) = "Unsuspended"
_Labels(6) = "Collected"
_Labels(7) = "Released from Hold"
_Labels(8) = "Modified"
_Labels(9) = "Discontinued for the Future"
_Labels(10) = "Verified"
_Labels(11) = "Modified And Verified"
_Labels(12) = "Hold Cancelled"
_ConditionalLabels(0) = ""
_ConditionalLabels(1) = "Conditional Cancelled"
_ConditionalLabels(2) = "Conditional Discontinued"
_ConditionalLabels(3) = "Conditional Suspended"
_ConditionalLabels(4) = "Conditional Unsuspended"
_ConditionalLabels(6) = "Conditional Collected"
_ConditionalLabels(7) = "Conditional Released from Hold"
_ConditionalLabels(8) = "Conditional Modified"
_ConditionalLabels(9) = "Conditional Discontinued for the Future"
_ConditionalLabels(10) = "Conditional Verified"
_ConditionalLabels(11) = "Conditional Modified And Verified"
_ConditionalLabels(12) = "Hold Conditional Cancelled"
_HeldLabels(0) = ""
_HeldLabels(1) = "Held Cancelled"
_HeldLabels(2) = "Held Discontinued"
_HeldLabels(3) = "Held Suspended"
_HeldLabels(4) = "Held Unsuspended"
_HeldLabels(6) = "Held Collected"
_HeldLabels(7) = "Held Released from Hold"
_HeldLabels(8) = "Held Modified"
_HeldLabels(9) = "Held Discontinued for the Future"
_HeldLabels(10) = "Held Verified"
_HeldLabels(11) = "Held Modified And Verified"
_HeldLabels(12) = "Hold Held Cancelled"
Dim str As String = ""
For i As Integer = 0 To 1000000
For j As Integer = -1 To 13
str = ComputeLabelRuss(j, False, False)
str = ComputeLabelRuss(j, True, False)
str = ComputeLabelRuss(j, False, True)
Next
Next
End Sub
Public Function ComputeLabelRuss(ByVal Action As Integer, ByVal Held As Boolean, ByVal Conditional As Boolean) As String
If Action < 0 OrElse Action > 12 Then Return ""
If Conditional Then Return _ConditionalLabels(Action)
If Held Then Return _HeldLabels(Action)
Return _Labels(Action)
End Function
Public Sub ComputeLabelsNew()
Dim str As String = ""
For i As Integer = 0 To 1000000
For j As Integer = -1 To 13
str = ComputeLabelNew(j, False, False)
str = ComputeLabelNew(j, True, False)
str = ComputeLabelNew(j, False, True)
Next
Next
End Sub
Public Function ComputeLabelNew(ByVal Action As Integer, ByVal IsHeld As Boolean, ByVal IsConditional As Boolean) As String
Dim Status As String = ""
Select Case Action
Case 0
Return ""
Case 1
Status = "Cancelled"
Case 2
Status = "Discontinued"
Case 3
Status = "Suspended"
Case 4
Status = "Unsuspended"
Case 6
Status = "Collected"
Case 7
Status = "Released from Hold"
Case 8
Status = "Modified"
Case 9
Status = "Discontinued for the Future"
Case 10
Status = "Verified"
Case 11
Status = "Modified And Verified"
Case 12
If IsConditional Then Return "Hold Conditional Cancelled"
If IsHeld Then Return "Hold Held Cancelled"
Return "Hold Cancelled"
Case Else
Return ""
End Select
If IsConditional Then Return "Conditional " & Status
If IsHeld Then Return "Held " & Status
Return Status
End Function
Sub ComputeLabelsErik()
Labels = New String() {"", "Cancelled", "Discontinued", "Suspended", "Unsuspended", "", "Collected", "Released from Hold", "Modified", "Discontinued for the Future", "Verified", "Modified And Verified", "Hold Cancelled"}
ConditionalLabels = New String() {"", "Conditional Cancelled", "Conditional Discontinued", "Conditional Suspended", "Conditional Unsuspended", "Conditional ", "Conditional Collected", "Conditional Released from Hold", "Conditional Modified", "Conditional Discontinued for the Future", "Conditional Verified", "Conditional Modified And Verified", "Hold Cancelled"}
HeldLabels = New String() {"", "Held Cancelled", "Held Discontinued", "Held Suspended", "Held Unsuspended", "Held ", "Held Collected", "Held Released from Hold", "Held Modified", "Held Discontinued for the Future", "Held Verified", "Held Modified And Verified", "Hold Cancelled"}
Dim str As String = ""
For i As Integer = 0 To 1000000
For j As Integer = -1 To 13
str = ComputeLabelErik(j, False, False)
str = ComputeLabelErik(j, True, False)
str = ComputeLabelErik(j, False, True)
Next
Next
End Sub
Public Function ComputeLabelErik(ByVal Action As Integer, ByVal Held As Boolean, ByVal Conditional As Boolean) As String
If Action < 0 OrElse Action > 12 Then Return ""
If Conditional Then Return ConditionalLabels(Action)
If Held Then Return HeldLabels(Action)
Return Labels(Action)
End Function
Sub ComputeLabelsErik2()
Dim str As String = ""
For i As Integer = 0 To 1000000
For j As Integer = 0 To 12
str = ComputeLabelErik2(j, False, False)
str = ComputeLabelErik2(j, True, False)
str = ComputeLabelErik2(j, False, True)
Next
For j As Integer = 1 To 2
str = ComputeLabelErik2(j, False, False)
str = ComputeLabelErik2(j, True, False)
str = ComputeLabelErik2(j, False, True)
Next
Next
End Sub
Public Function ComputeLabelErik2(ByVal Action As Integer, ByVal Held As Boolean, ByVal Conditional As Boolean) As String
If Conditional Then Return ConditionalLabels(Action)
If Held Then Return HeldLabels(Action)
Return Labels(Action)
End Function
Public Sub ComputeLabelsOriginal()
Dim str As String = ""
For i As Integer = 0 To 1000000
For j As Integer = -1 To 13
str = ComputeLabelOriginal(j, False, False)
str = ComputeLabelOriginal(j, True, False)
str = ComputeLabelOriginal(j, False, True)
Next
Next
End Sub
Public Function ComputeLabelOriginal(ByVal Action As Integer, ByVal bIsHeld As Boolean, _
ByVal bIsConditional As Boolean) As String
Dim strReprintLabel As String = ""
Dim strOrderActionPrefix As String = ""
If (bIsHeld) Then
strOrderActionPrefix = "Held "
End If
If (bIsConditional) Then
strOrderActionPrefix = "Conditional "
End If
Select Case Action
Case 0 ' Normal Order
strReprintLabel = ""
Case 1
strReprintLabel = strOrderActionPrefix & "Order Cancelled"
Case 2
strReprintLabel = strOrderActionPrefix & "Order Discontinued"
Case 3
strReprintLabel = strOrderActionPrefix & "Order Suspended"
Case 4
strReprintLabel = strOrderActionPrefix & "Order Unsuspended"
Case 6
strReprintLabel = strOrderActionPrefix & "Order Collected"
Case 7
strReprintLabel = strOrderActionPrefix & "Order Released from Hold"
Case 8
strReprintLabel = strOrderActionPrefix & "Order Modified"
Case 9
strReprintLabel = strOrderActionPrefix & "Order Discontinued for the Future"
Case 10
strReprintLabel = strOrderActionPrefix & "Order Verified"
Case 11
strReprintLabel = strOrderActionPrefix & "Order Modified And Verified"
Case 12
strReprintLabel = "Hold " & strOrderActionPrefix & "Order Cancelled"
Case Else
strReprintLabel = ""
End Select
Return strReprintLabel
End Function
Sub ComputeLabelsSteve2()
Dim str As String = ""
For i As Integer = 0 To 1000000
For j As Integer = -1 To 13
str = ComputeLabelSteve2(j, False, False)
str = ComputeLabelSteve2(j, True, False)
str = ComputeLabelSteve2(j, False, True)
Next
Next
End Sub
Public Function ComputeLabelSteve2(ByVal Action As Integer, ByVal IsHeld As Boolean, ByVal IsConditional As Boolean) As String
Select Case Action
Case 0
Return ""
Case 1
If IsConditional Then Return "Conditional Cancelled"
If IsHeld Then Return "Held Cancelled"
Return "Cancelled"
Case 2
If IsConditional Then Return "Conditional Discontinued"
If IsHeld Then Return "Held Discontinued"
Return "Discontinued"
Case 3
If IsConditional Then Return "Conditional Suspended"
If IsHeld Then Return "Held Suspended"
Return "Suspended"
Case 4
If IsConditional Then Return "Conditional Unsuspended"
If IsHeld Then Return "Held Unsuspended"
Return "Unsuspended"
Case 6
If IsConditional Then Return "Conditional Collected"
If IsHeld Then Return "Held Collected"
Return "Collected"
Case 7
If IsConditional Then Return "Conditional Released from Hold"
If IsHeld Then Return "Held Released from Hold"
Return "Released from Hold"
Case 8
If IsConditional Then Return "Conditional Modified"
If IsHeld Then Return "Held Modified"
Return "Modified"
Case 9
If IsConditional Then Return "Conditional Discontinued for the Future"
If IsHeld Then Return "Held Discontinued for the Future"
Return "Discontinued for the Future"
Case 10
If IsConditional Then Return "Conditional Verified"
If IsHeld Then Return "Held Verified"
Return "Verified"
Case 11
If IsConditional Then Return "Conditional Modified And Verified"
If IsHeld Then Return "Held Modified And Verified"
Return "Modified And Verified"
Case 12
If IsConditional Then Return "Hold Conditional Cancelled"
If IsHeld Then Return "Hold Held Cancelled"
Return "Hold Cancelled"
Case Else
Return ""
End Select
End Function
Sub ComputeLabelsSteve()
Dim str As String = ""
For i As Integer = 0 To 1000000
For j As Integer = -1 To 13
str = ComputeLabelSteve(j, False, False)
str = ComputeLabelSteve(j, True, False)
str = ComputeLabelSteve(j, False, True)
Next
Next
End Sub
Public Function ComputeLabelSteve(ByVal Action As Integer, ByVal IsHeld As Boolean, ByVal IsConditional As Boolean) As String
Select Case Action
Case 0
Return ""
Case 1
If IsConditional Then
Return "Conditional Cancelled"
ElseIf IsHeld Then
Return "Held Cancelled"
Else
Return "Cancelled"
End If
Case 2
If IsConditional Then
Return "Conditional Discontinued"
ElseIf IsHeld Then
Return "Held Discontinued"
Else
Return "Discontinued"
End If
Case 3
If IsConditional Then
Return "Conditional Suspended"
ElseIf IsHeld Then
Return "Held Suspended"
Else
Return "Suspended"
End If
Case 4
If IsConditional Then
Return "Conditional Unsuspended"
ElseIf IsHeld Then
Return "Held Unsuspended"
Else
Return "Unsuspended"
End If
Case 6
If IsConditional Then
Return "Conditional Collected"
ElseIf IsHeld Then
Return "Held Collected"
Else
Return "Collected"
End If
Case 7
If IsConditional Then
Return "Conditional Released from Hold"
ElseIf IsHeld Then
Return "Held Released from Hold"
Else
Return "Released from Hold"
End If
Case 8
If IsConditional Then
Return "Conditional Modified"
ElseIf IsHeld Then
Return "Held Modified"
Else
Return "Modified"
End If
Case 9
If IsConditional Then
Return "Conditional Discontinued for the Future"
ElseIf IsHeld Then
Return "Held Discontinued for the Future"
Else
Return "Discontinued for the Future"
End If
Case 10
If IsConditional Then
Return "Conditional Verified"
ElseIf IsHeld Then
Return "Held Verified"
Else
Return "Verified"
End If
Case 11
If IsConditional Then
Return "Conditional Modified And Verified"
ElseIf IsHeld Then
Return "Held Modified And Verified"
Else
Return "Modified And Verified"
End If
Case 12
If IsConditional Then
Return "Hold Conditional Cancelled"
ElseIf IsHeld Then
Return "Hold Held Cancelled"
Else
Return "Hold Cancelled"
End If
Case Else
Return ""
End Select
End Function
Sub ComputeLabelsChoose()
Dim str As String = ""
For i As Integer = 0 To 1000000
For j As Integer = -1 To 13
str = ComputeLabelChoose(j, False, False)
str = ComputeLabelChoose(j, True, False)
str = ComputeLabelChoose(j, False, True)
Next
Next
End Sub
Public Function ComputeLabelChoose(ByVal Action As Integer, ByVal IsHeld As Boolean, ByVal IsConditional As Boolean) As String
Dim Status As String = ""
Select Case Action
Case 0, 5
Return ""
Case 1 To 11
Status = Choose(Action, "Cancelled", "Discontinued", "Suspended", _
"Unsuspended", "Collected", "Released from Hold", "Modified", _
"Discontinued for the Future", "Verified", "Modified And Verified")
Case 12
If IsConditional Then
Return "Hold Conditional Cancelled"
ElseIf IsHeld Then
Return "Hold Held Cancelled"
Else
Return "Hold Cancelled"
End If
Case Else
Return ""
End Select
If IsConditional Then Return "Conditional " & Status
If IsHeld Then Return "Held " & Status
Return Status
End Function
Sub ComputeLabelsBigArray()
OrderLabelsBigArray = New String() {"", "Cancelled", "Discontinued", "Suspended", "Unsuspended", "", "Collected", "Released from Hold", "Modified", "Discontinued for the Future", "Verified", "Modified And Verified", "Hold Cancelled", _
"", "Conditional Cancelled", "Conditional Discontinued", "Conditional Suspended", "Conditional Unsuspended", "Conditional ", "Conditional Collected", "Conditional Released from Hold", "Conditional Modified", "Conditional Discontinued for the Future", "Conditional Verified", "Conditional Modified And Verified", "Hold Cancelled", _
"", "Held Cancelled", "Held Discontinued", "Held Suspended", "Held Unsuspended", "Held ", "Held Collected", "Held Released from Hold", "Held Modified", "Held Discontinued for the Future", "Held Verified", "Held Modified And Verified", "Hold Cancelled"}
Dim str As String = ""
For i As Integer = 0 To 1000000
For j As Integer = -1 To 13
str = ComputeLabelChoose(j, False, False)
str = ComputeLabelChoose(j, True, False)
str = ComputeLabelChoose(j, False, True)
Next
Next
End Sub
Public Function ComputeLabelBigArray(ByVal Action As Integer, ByVal IsHeld As Boolean, ByVal IsConditional As Boolean) As String
If Action < 0 OrElse Action >= 13 Then Return ""
Return OrderLabelsBigArray(Action - IsConditional * 13 - (IsHeld AndAlso Not IsConditional) * 26)
End Function
End Module
Now if I made another mistake, someone can help find it.
Steve Wortham already said the most important thing: the only inefficient part in your function is the string concatenation. If you really need that function to be fast, disregard source code length and expand every case explicitly, so you can avoid concatenating strings.
I would like to add one thing: Using an array won't help you optimize the function. .NET arrays are not C arrays, they are complex objects that must be constructed, filled with data and destroyed.
Any thoughts on this version?
Public Shared Function ComputeLabel(ByVal Action As Integer, ByVal IsHeld As Boolean, ByVal IsConditional As Boolean) As String
Dim Status As String = ""
Select Case Action
Case 0
Return ""
Case 1
Status = "Cancelled"
Case 2
Status = "Discontinued"
Case 3
Status = "Suspended"
Case 4
Status = "Unsuspended"
Case 6
Status = "Collected"
Case 7
Status = "Released from Hold"
Case 8
Status = "Modified"
Case 9
Status = "Discontinued for the Future"
Case 10
Status = "Verified"
Case 11
Status = "Modified And Verified"
Case 12
If IsConditional Then Return "Hold Conditional Cancelled"
If IsHeld Then Return "Hold Held Cancelled"
Return "Hold Cancelled"
Case Else
Return ""
End Select
If IsConditional Then Return "Conditional " & Status
If IsHeld Then Return "Held " & Status
Return Status
End Function
or this?
Public Shared Function ComputeLabel(ByVal Action As Integer, ByVal IsHeld As Boolean, ByVal IsConditional As Boolean) As String
Dim Status As String = ""
Select Case Action
Case 0, 5
Return ""
Case 1 To 11
Status = Choose(Action, "Cancelled", "Discontinued", "Suspended", _
"Unsuspended", "Collected", "Released from Hold", "Modified", _
"Discontinued for the Future", "Verified", "Modified And Verified")
Case 12
If IsConditional Then
Return "Hold Conditional Cancelled"
ElseIf IsHeld Then
Return "Hold Held Cancelled"
Else
Return "Hold Cancelled"
End If
Case Else
Return ""
End Select
If IsConditional Then Return "Conditional " & Status
If IsHeld Then Return "Held " & Status
Return Status
End Function
I suspect the Choose function might be really slow...
And I guess the original function will still probably be clearer to the people maintaining the code than any of these other options. Sigh.
加载中,请稍侯......
精彩评论