4
\$\begingroup\$

I have a simple quantity display function that runs for every product displayed in our catalog. The MaxQtyDisplay value is set in our web.config file. Part of me thinks the function is as streamlined as possible and part of me thinks it can be improved upon somewhere. Because this function is called many, many times in our web application I would like it be optimized as best it could.

What could be done to improve it?

Private Shared Function CalculateQtyDisplay(ByVal x As Double, ByVal y As Double) As Double
    Dim q As Double = x - y
    If q > 0 And q <= My.Settings.MaxQtyDisplay Then
        Return q
    ElseIf q > My.Settings.MaxQtyDisplay Then
        Return My.Settings.MaxQtyDisplay
    Else
        Return 0
    End If
End Function
\$\endgroup\$
1
  • \$\begingroup\$ Are the values involved really doubles? Using Integers would help. \$\endgroup\$
    – Bill
    Commented Jun 11, 2011 at 14:19

1 Answer 1

5
\$\begingroup\$

I'm sure there will be elements of your application that are of much more concern, performance-wise, than this, but it could certainly be a little more clear, or maybe a little less superfluous...

  • Evaluation of and work within conditions is only required if q > 0
  • You really want to use AndAlso to apply short-circuiting in the first condition
  • q > MaxQtyDisplay is redundant since this condition wouldn't hit otherwise (forgetting negative quantities but we address that by addressing my first point)
  • Else is likewise not required.
  • Only get the value of MaxQtyDisplay from the settings context once (negligible in terms of performance really, since settings are loaded from file into memory on request)
  • Naming variables with single character identifiers doesn't optimise anything

So, for example, you can correct these as such:

Private Function CalculateQtyDisplay(ByVal x As Double, ByVal y As Double) As Double
    Dim result = 0.0
    Dim desiredQuantity As Double = x - y
    Dim maxQuantity = My.Settings.MaxQtyDisplay
    If (desiredQuantity > 0) Then
        If (desiredQuantity <= maxQuantity) Then
            result = maxQuantity
        Else
            result = desiredQuantity
        End If
    End If
    Return result
End Function

Any other modifications to this code are simply going to be aesthetic and preferential really, as far as I can see. For instance you could do away with maintaining result, this goes against what I recommend in another answer but it clearly boils down to context):

Dim desiredQuantity As Double = x - y
Dim maxQuantity = My.Settings.MaxQtyDisplay
If (desiredQuantity > 0) Then
    If (desiredQuantity <= maxQuantity) Then
        Return maxQuantity
    End If
    Return desiredQuantity
End If
Return 0

A modification that comes to mind which could potentially impact performance, if anything might, is to use the IIf function available to VB.NET (both TruePart and FalsePart of this call are evaluated, though I highly doubt you'll ever have any noticeable degradation with this method anyway, even if you used this method):

Dim quantity As Double = x - y
Dim maxQtyDisplay = 1800 'My.Settings.MaxQtyDisplay
If (quantity > 0) Then
    Return IIf(quantity <= maxQtyDisplay, quantity, maxQtyDisplay)
End If
Return 0
\$\endgroup\$
1
  • \$\begingroup\$ WOW. I didn't even think of the solution you proposed. Very well written explanation as well. I actually use AndAlso in other functions I have, but didn't even think of using it here. \$\endgroup\$
    – NinjaBomb
    Commented Apr 25, 2011 at 18:57

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.