4
\$\begingroup\$

I have a Calculation class. It is persisted in the DB and users can modify it.

The aim of the CompareWith function is to display to the users which parameters have been changed.

A good part of the function is just the same code duplicated. I have been thinking about a way to abstract it out in a function, but have not been able to figure it out.

How can I improve this function?

Public Class Calculation
    Public Property ValueCalculation As String
    Public Property LowLowLowLimitCalculation As String
    Public Property LowLowLimitCalculation As String
    Public Property LowLimitCalculation As String
    Public Property TargetCalculation As String
    Public Property HighLimitCalculation As String
    Public Property HighHighLimitCalculation As String
    Public Property HighHighHighLimitCalculation As String

    Public Function CompareWith(comparedCalculation As Calculation) As CalculationError
        Dim resultStatus As New CalculationError
        If comparedCalculation Is Nothing Then
            Return CalculationError.NotCreated
        End If

        If Not (ValueCalculation = comparedCalculation.ValueCalculation) Then
            resultStatus = resultStatus Or CalculationError.Value
        End If
        If Not (LowLowLowLimitCalculation = comparedCalculation.LowLowLowLimitCalculation) Then
            resultStatus = resultStatus Or CalculationError.LowLowLowLimit
        End If
        If Not (LowLowLowLimitCalculation = comparedCalculation.LowLowLowLimitCalculation) Then
            resultStatus = resultStatus Or CalculationError.LowLowLimit
        End If
        If Not (LowLimitCalculation = comparedCalculation.LowLimitCalculation) Then
            resultStatus = resultStatus Or CalculationError.LowLimit
        End If
        If Not (TargetCalculation = comparedCalculation.TargetCalculation) Then
            resultStatus = resultStatus Or CalculationError.Target
        End If
        If Not (HighHighHighLimitCalculation = comparedCalculation.HighHighHighLimitCalculation) Then
            resultStatus = resultStatus Or CalculationError.HighHighHighLimit
        End If
        If Not (HighHighLimitCalculation = comparedCalculation.HighHighLimitCalculation) Then
            resultStatus = resultStatus Or CalculationError.HighHighLimit
        End If
        If Not (HighLimitCalculation = comparedCalculation.HighLimitCalculation) Then
            resultStatus = resultStatus Or CalculationError.HighLimit
        End If
        Return resultStatus
    End Function
End Class

<Flags>
Public Enum CalculationError
    NotCreated = 1
    Value = 2
    LowLowLowLimit = 4
    LowLowLimit = 8
    LowLimit = 16
    HighLimit = 32
    HighHighLimit = 64
    HighHighHighLimit = 128
    Target = 256
End Enum

Example of use:

Public Sub Main()
    Dim foo As New Calculation() With {.ValueCalculation = "1+1"}
    Dim bar As new Calculation() With {.ValueCalculation = "2+2"}
    Console.WriteLine(foo.CompareWith(bar).ToString())
    'Output: Value
End Sub
\$\endgroup\$
  • \$\begingroup\$ would you please provide the entire class? \$\endgroup\$ – Malachi Aug 1 '18 at 13:31
  • 1
    \$\begingroup\$ This is the entire thing. I have added an example of how it is used for clarity \$\endgroup\$ – Maxime Aug 1 '18 at 13:38
  • 1
    \$\begingroup\$ HighHighHighLimitCalculation - what happened to this property (and other similar ones)? What kind of convention is this? There seem to be a much bigger problem with your class than the duplicated code. \$\endgroup\$ – t3chb0t Aug 8 '18 at 16:17
  • \$\begingroup\$ If the Target is higher than the HighHighHighLimit, what does any of it mean? \$\endgroup\$ – Mast Aug 10 '18 at 10:58
  • \$\begingroup\$ @Mast Nothing at all. It's just about comparing string properties, and seeing if they are equal to the property with the same name in an other object. If they are not, add the flag with the name of the property. The properties are just strings, which happen to have Calculation in the name. \$\endgroup\$ – Maxime Aug 10 '18 at 11:10

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Browse other questions tagged or ask your own question.