HiveBrain v1.2.0
Get Started
← Back to all entries
patternMinor

Display catalog product quantity

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
quantitydisplayproductcatalog

Problem

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 My.Settings.MaxQtyDisplay
    Else
        Return 0
    End If
End Function

Solution

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

Code Snippets

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
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
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

Context

StackExchange Code Review Q#2093, answer score: 5

Revisions (0)

No revisions yet.