Hi, all,
Yesterday, I sent this article on the technic of eliminating select-case structure in the code... and I received a feedback.
the feedback says, okay, all you said and shown are good, but we have some kind of code in this structure:
Select Case drivingLicense.TransactionType
Case DringLicenseTransaction.Original
Return Original(drivingLicense)
Case DringLicenseTransaction.Renew
Return Renew(drivingLicense)
Case DringLicenseTransaction.Replace
Return Replace(drivingLicense)
Case Else
Return "not capable to handle the transaction"
End Select
As all know, we have about 30 title transaction types and about 50 Driving License Transaction Types. etc. The case structure can be very long, hence the cyclomatic complexity of the method could be very high. different from the example you give, in this scenario they all taking the same transaction object, overloading would not work. do you have some advice for us.
this is a very good question, I appreciate the person who brought this up and offered this opportunity for me share another technic on the topic... I spent some time last night made another POC to show case the technic.
supposedly we have a enum as following
Public Enum DringLicenseTransaction
Replace
Renew
Original
End Enum
and we have a BE defined as following
Public Class DrivingLicenseBE
Public Property TransactionType As DringLicenseTransaction
Public Sub New(ByVal transactionType As DringLicenseTransaction)
Me.TransactionType = transactionType
End Sub
End Class
then we have a BO class defined as following
Public Class DrivingLicenseBO
Private Function Renew(ByVal drivingLicense As DrivingLicenseBE)
Return "renew In progress"
End Function
Private Function Replace(ByVal drivingLicense As DrivingLicenseBE)
Return "Replace In progress"
End Function
Private Function Original(ByVal drivingLicense As DrivingLicenseBE)
Return "Replace In progress"
End Function
Public Function ProcessWithSelectCase(ByVal drivingLicense As DrivingLicenseBE) As String
Select Case drivingLicense.TransactionType
Case DringLicenseTransaction.Original
Return Original(drivingLicense)
Case DringLicenseTransaction.Renew
Return Renew(drivingLicense)
Case DringLicenseTransaction.Replace
Return Replace(drivingLicense)
Case Else
Return "not capable to handle the transaction"
End Select
End Function
End Class
this public method will be called with a DrivingLicenseBE instance, and based on the value of its TransactionType property it calls different method...
in this case, it is fine, because I only have 3 values in DringLicenseTransaction Enum definition, if you have 20, 30, 40 or even more, your select case statement would be long and the cyclomatic complexity would be very high. in our code base, we had a method with cyclomatic complexity more than one thousand because of this structure.
well, there is a way to make it one line here. are you interested to know how?
before we touch the method, we need to do some ground work on DrivingLicenseBO. But I promise you the ground work I am going to do is very straightforward, no "if"s no, "select-case" for sure.
1) let's get started by adding a delegate to the class
Private Delegate Function ProcessDrivingLicenseTransaction(ByVal drivingLicense As DrivingLicenseBE) As String
2) let's define a Dictionary of the delegate as following:
Private ReadOnly mDrivingLicenseProcessers As Dictionary(Of DringLicenseTransaction, ProcessDrivingLicenseTransaction)
3) last step add a constructor to populate the dictionary:
Public Sub New()
mDrivingLicenseProcessers = New Dictionary(Of DringLicenseTransaction, ProcessDrivingLicenseTransaction)
mDrivingLicenseProcessers.Add(DringLicenseTransaction.Original, New ProcessDrivingLicenseTransaction(AddressOf Original))
mDrivingLicenseProcessers.Add(DringLicenseTransaction.Renew, New ProcessDrivingLicenseTransaction(AddressOf Renew))
mDrivingLicenseProcessers.Add(DringLicenseTransaction.Replace, New ProcessDrivingLicenseTransaction(AddressOf Replace))
End Sub
as I promised, no "if", no "select-case", right?
now let's see what we can do for the select case structure:
If mDrivingLicenseProcessers.ContainsKey(drivingLicense.TransactionType) Then
Return mDrivingLicenseProcessers(drivingLicense.TransactionType).Invoke(drivingLicense)
Else
Return "not capable to handle the transaction"
End If
The if statement above can be used to replace the select case structure no matter how big is the case structure. the really important line is only one:
Return mDrivingLicenseProcessers(drivingLicense.TransactionType).Invoke(drivingLicense)
from the code above you can find that one case clause is converted to one add method call to the dictionary. the dynamic function call become static dictionary population. in most of case this will lead to less defect, easy to debug...
in our code base, we deployed this technic in refectorying one of the most complex method in the code, we managed to reduce the cyclomatic complexity from over 1100 to less than 20. the magic word is Delegate.
Again, if you have any comments, questions please feel free to voice out or reach out to me.
This is what I believe: If there is something very tedious to do, there must be a better way, it is up to you to find it. Programming can be enjoyable if we do it right! Enjoy it.
No comments:
Post a Comment