Reducing Cyclomatic Complexity in a select

89 Views Asked by At

Good day coding godz,

I am trying to clean up some code and was wondering how to reduce the Cylcomatic Complexity on a method I have created. This method is used during an Import of a CSV file. One of the fields in the CSV file is a license type that is a string (ie "BFI Supervised Lender - Website #2 License") I am converting this to a Integer (1,2,3 or 4) that will be saved to the Database to reference the type of industry based on license type.

Below is my method. Would appreciate some tips for a shade tree VB.NET coder...

Private Function CheckIndustryType(LicName)
    Dim VAR_IndType As Integer

    Select Case Text_LicName
        Case "BFI Level I Check Cashing - Branch License"
            VAR_IndType = 3
        Case "BFI Level II Check Cashing - Branch Certificate"
            VAR_IndType = 3
        Case "BFI Supervised Lender - Branch License"
            VAR_IndType = 1
        Case "BFI Deferred Presentment - Branch License"
            VAR_IndType = 3
        Case "BFI Supervised Lender - Website #1 License"
            VAR_IndType = 1
        Case "BFI Supervised Lender - Website #2 License"
            VAR_IndType = 1
        Case "BFI Supervised Lender - Website #3 License"
            VAR_IndType = 1
        Case "BFI Supervised Lender - Website #4 License"
            VAR_IndType = 1
        Case "BFI Supervised Lender - Website #5 License"
            VAR_IndType = 1
        Case "BFI Supervised Lender - Website #6 License"
            VAR_IndType = 1
        Case "BFI Level II Check Cashing - Company License"
            VAR_IndType = 3
        Case "BFI Level I Check Cashing - Company License"
            VAR_IndType = 3
        Case "fI Branch Mortgage Lender/Servicer"
            VAR_IndType = 2
        Case "BFI Branch Mortgage Lender/Servicer - Other Trade Name #1"
            VAR_IndType = 2
        Case "BFI Branch Mortgage Lender/Servicer - Other Trade Name #2"
            VAR_IndType = 2
        Case "BFI Branch Mortgage Lender/Servicer - Other Trade Name #3"
            VAR_IndType = 2
        Case "BFI Branch Mortgage Lender/Servicer - Other Trade Name #4"
            VAR_IndType = 2
        Case "BFI Branch Mortgage Lender/Servicer - Other Trade Name #5"
            VAR_IndType = 2
        Case "BFI Branch Mortgage Lender/Servicer - Other Trade Name #6"
            VAR_IndType = 2
        Case "BFI Mortgage Lender / Servicer License"
            VAR_IndType = 2
        Case "BFI Mortgage Lender/Servicer License - Other Trade Name #1"
            VAR_IndType = 2
        Case "BFI Mortgage Lender/Servicer License - Other Trade Name #2"
            VAR_IndType = 2
        Case "BFI Mortgage Lender/Servicer License - Other Trade Name #3"
            VAR_IndType = 2
        Case "BFI Mortgage Lender/Servicer License - Other Trade Name #4"
            VAR_IndType = 2
        Case "BFI Mortgage Lender/Servicer License - Other Trade Name #5"
            VAR_IndType = 2
        Case "BFI Mortgage Lender/Servicer License - Other Trade Name #6"
            VAR_IndType = 2
        Case Else
            VAR_IndType = 4
    End Select

    Return VAR_IndType 
End Function
3

There are 3 best solutions below

2
jonyfries On BEST ANSWER

I would use a dictionary in a situation like this. It reduces the number of independent paths to 2, the second is necessary since you're using a default value of course.

Module Module1
    Dim industryTypeMap As New Dictionary(Of String, Int32) From {{"BFI Level I Check Cashing - Branch License", 3},
                                                                           {"BFI Level II Check Cashing - Branch Certificate", 3},
                                                                           {"BFI Supervised Lender - Branch License", 1}}

    Sub Main()
        Console.WriteLine(CheckIndustryType("BFI Supervised Lender - Branch License"))
        Console.WriteLine(CheckIndustryType("BFI Level II Check Cashing - Branch Certificate"))
        Console.WriteLine(CheckIndustryType("Other"))
    End Sub

    Private Function CheckIndustryType(LicName As String)
        Dim industryInt As Int32
        If industryTypeMap.TryGetValue(LicName, industryInt) Then
            Return industryInt
        Else
            Return 4
        End If
    End Function
End Module

This outputs:

1
3
4

You could also define the dictionary in the function to maintain all of the code together but that would obviously run slower if you're calling the function repeatedly.

As per the comment below - Ideally you would place the actual mapping in an external item that could be updated without recompiling the code (eg a config file or a database).

4
Mary On

I shortened the number of cases by using StartsWith and Contains with Select Case True.

Private Function CheckIndustryType(LicName As String) As Integer
    Dim VAR_IndType As Integer

    Select Case True
        Case LicName.Contains("Check Cashing")
            VAR_IndType = 3
        Case LicName.StartsWith("BFI Supervised Lender")
            VAR_IndType = 1
        Case LicName.StartsWith("BFI Deferred Presentment")
            VAR_IndType = 3
        Case LicName.StartsWith("fI")
            VAR_IndType = 2
        Case LicName.StartsWith("BFI Branch Mortgage Lender")
            VAR_IndType = 2
        Case Else
            VAR_IndType = 4
    End Select

    Return VAR_IndType
End Function
0
Joshua Frank On

A long list of long strings is risky, because any change is quite likely to introduce a typo and then the strings don't match. I would probably make an Enum:

Enum LicenseType
  BFILevelICheckCashingBranchLicense
  BFILevelIICheckCashingBranchCertificate
  BFISupervisedLenderBranchLicense
  ...
End Enum

and then test against that value. To reduce the complexity, I would make an attribute like:

Class VarIndTypeAttribute 
  Inherits System.Attribute

    Public VarIndType As Integer
    Sub New(varIndType As Integer
        Me.VarIndType = varIndType
    End Sub

End Class

and then the enum becomes:

Enum
  <VarIndType(3)>
  BFILevelICheckCashingBranchLicense
  <VarIndType(3)>
  BFILevelIICheckCashingBranchCertificate
  <VarIndType(1)>
  BFISupervisedLenderBranchLicense
  ...
End Enum

and then you make a method like

    <Extension>
    Function ToVarIndType(enumValue As [Enum]) As Integer
        Dim att = enumValue.GetAttributeOfType(Of VarIndTypeAttribute)()
        Return If(att IsNot Nothing, att.VarIndType, 0)
    End Function

and then

    Dim valueEnum As LicenseType = ...
    Dim VarIndType = valueEnum.ToVarIndType

and you don't need the lookup function at all!

You probably want VarIndType to be an Enum as well, so you don't have magic values like 0 and 3 throughout the app.