I spent a good part of today refactoring some code I’ve inherited. It’s a VB.Net wrapper around some commercial address verification software. Two examples of the type of functionality the software provides are address correction and address formatting.
Here is a simplified version of three of the classes I was given to work with: Address, CorrectedAddress and FormattedAddress. An Address is what you would provide to the verification software and the other two classes would be returned from either the address correction or address formatting functionality.
Public Class Address Public AddressLine As String Public City As String Public Province As String Public PostalCode As String Public Country As String End Class
Public Class CorrectedAddress : Inherits Address Public Status As CorrectedAddress.Status Public Message As String Public Enum Status Valid Invalid Corrected [Error] End Enum End Class
Public Class FormattedAddress Public AddressLineOne As String Public AddressLineTwo As String Public AddressLineThree As String Public Status As FormattedAddress.Status Public Message As String Public Enum Status Valid Invalid [Error] End Enum End Class
Note that CorrectedAddress inherits from Address, but FormattedAddress does not. Both CorrectedAddress and FormattedAddress have a Status and Message properties (implemented as public fields above only for brevity). The Status properties both return similar, but not identical enumerations. In the actual code there were six classes like these two, but I am focusing only on these two for simplicity.
The whole thing didn’t seem quite right to me, and I struggled a bit to put my finger on how this should have been implemented. But then it hit me. CorrectedAddress isn’t really an Address. It’s a result of an address correction (that happens to have an address). “Is A” versus “Has A”. Remember the oft quoted Object-Oriented design principle:
Favour Composition over Inheritance
So I pulled the Address class out of the inheritance hierarchy, renamed CorrectedAddress and FormattedAddress to CorrectedResult and FormattedResult respectively and even threw in some Generics stuff to deal with the similar but different Status enumerations.
The result (the Address class was left as-is):
Public MustInherit Class Result(Of T As Structure) Public Status As T Public Message As String End Class
Public Class CorrectedResult : Inherits Result(Of CorrectedResultStatus) Public Address As Address Public Enum CorrectedResultStatus Valid Invalid Corrected [Error] End Enum End Class
Public Class FormattedResult : Inherits Result(Of FormattedResultStatus Public AddressLineOne As String Public AddressLineTwo As String Public AddressLineThree As String Public Enum FormattedResultStatus Valid Invalid [Error] End Enum End Class
Much better!