How to avoid code duplication in ordering system

Question

Here is the piece of order system.
And this is hierarchy of order classes in this system

Public MustInherit Class BaseItem
    Public Property Id() As Guid
    Public Property Sku() As String
    Public Property Quantity() As String
    Public Property TransactionId() As Integer?
End Class

Public Class SoOrderItem
    Inherits BaseItem

    Public Property BatchNumber() As String
    Public Property Lottable1() As String
    Public Property Lottable2() As String
    Public Property Lottable3() As String
    Public Property Lottable4() As String

End Class

Public Class AsnOrderItem
    Inherits BaseItem

   Public Property AdditionalInfo() As String

End Class

Public Class BaseOrder
    Public Property OrderReference() As String
    Public Property PoNumber() As String
    Public Property CustomerVat() As String
    Public Property CustomerReference() As String
    Public Property TransactionType() As String
End Class

Public Class SoOrder
    Inherits BaseOrder

    Public Property ConsigneeAddress() As String
    Public Property ConsigneeContact() As String
    Public Property ConsigneeName() As String
    Public Property OrderItems() As New List(Of SoOrderItem)
End Class

Public Class AsnOrder
    Inherits BaseOrder

    Public Property Account() As Account
    Public Property SupplierName() As String
    Public Property OrderItems() As New List(Of AsnOrderItem)
End Class

And here is confirm classes for them

Public Class SoConfirmation
     Private _itransactionService As ITransactionService

     Private Sub AssignNewTransactionsToOrderItems(soOrders As IEnumerable(Of SoOrder))

        For Each order In soOrders
            For Each item In order.OrderItems
                If (Not item.TransactionId.HasValue) Then
                    Dim newTransactionId = 
_itransactionService.GenerateTransactionId()
   _itransactionService.AssignTransactionToOrderItem(order.OrderReference, item.Sku, item.Id, newTransactionId)
                    item.TransactionId = newTransactionId
                End If
            Next
        Next
    End Sub

    Private Sub FilterForConfirmation(soOrders As IEnumerable(Of SoOrder))

        For Each order In soOrders
            order.OrderItems.RemoveAll(Function(i) _itransactionService.IsTransactionProcessedSuccesfully(order.OrderReference, i.Id))
        Next
    End Sub
End Class

Public Class AsnConfirmation
     Private _itransactionService As ITransactionService

  Private Sub AssignNewTransactionsToOrderItems(asnOrders As IEnumerable(Of AsnOrder))

        For Each asn In asnOrders
            For Each item In asn.OrderItems
                If (Not item.TransactionId.HasValue) Then
                    Dim newTransactionId = _itransactionService.GenerateTransactionId()

_itransactionService.AssignTransactionToOrderItem(asn.OrderReference, item.Sku, item.Id, newTransactionId)
                    item.TransactionId = newTransactionId
                End If
            Next
        Next
    End Sub

    Private Sub FilterForConfirmation(asnOrders As IEnumerable(Of AsnOrder))

        For Each order In asnOrders
            order.OrderItems.RemoveAll(Function(i) _itransactionService.IsTransactionProcessedSuccesfully(order.OrderReference, i.Id))
        Next
    End Sub
End Class

As you can see confirmation classes have a lot of duplcited code and i don't know how to avoid it ... Will be appreciate for any help.

Answer 1

You are right that you have code duplication where no duplication should be. As far as I can see from the code you have posted, nothing really changes if you would write a class that would handle it as a BaseOrder, since all the properties you are referring to are available on the BaseOrder (except for OrderItems, however this could be retrieved through an extra method implementing it)

So you could write your class more like

Public MustInherit Class BaseOrderConfirmation
    Private _itransactionService As ITransactionService

    Protected MustOverride Function GetOrderItems(order as BaseOrder) As IEnumerable(Of BaseItem)

    Protected Sub AssignNewTransactionsToOrderItems(orders As List(Of BaseOrder))
        For Each order In orders
            For Each item In GetOrderItems(order)
                If (Not item.TransactionId.HasValue) Then
                    Dim newTransactionId = _itransactionService.GenerateTransactionId()    
                    _itransactionService.AssignTransactionToOrderItem(order.OrderReference, item.Sku, item.Id, newTransactionId)
                    item.TransactionId = newTransactionId
                End If
            Next
        Next
    End Sub

    Protected Sub FilterForConfirmation(orders As IEnumerable(Of BaseOrder))
        For Each order In orders 
            GetOrderItems(order).RemoveAll(Function(i) _itransactionService.IsTransactionProcessedSuccesfully(order.OrderReference, i.Id))
        Next
    End Sub
End Class

This would now be an abstract class (ie: a class that cannot be instantiated), from which the GetOrderItems method has to be implemented still.

Also note, that this change means that all methods now changed from Private modifier to the Protected modifier

In the implementing classes you can then implement something like:

Public Class SoOrderConfirmation
    Inherits BaseOrderConfirmation

    Protected Overrides Function GetOrderItems(order as BaseOrder) as List(Of BaseItem)
        return DirectCast(order, SoOrder).OrderItems
    End Function
End Class

Preferably though, the GetOrderItems method shouldn't return anything that can be modified. I wish I could create it as an IEnumerable(Of BaseItem) instead, however, that would not work with your FilterForConfirmation method.

As I do not know how you intend to filter, I chose to keep the List(Of BaseItem). The filter also doesn't seem to filter, but rather seems to remove items from that list, so without knowing how you are using it, I chose to stick with List(Of BaseItem).

Answer 2

Here is my code after @Icepickle suggestons

Public MustInherit Class BaseOrder
    Public Property OrderReference() As String
    Public Property PoNumber() As String
    Public Property CustomerVat() As String
    Public Property CustomerReference() As String
    Public Property TransactionType() As String
    Public MustOverride ReadOnly Property OrderItems() As IEnumerable(Of BaseOrderItem)

End Class

Public Class AsnOrder
    Inherits BaseOrder

    Public Property SupplierName() As String
    Public Overrides ReadOnly Property OrderItems() As IEnumerable(Of BaseOrderItem)
        Get
            Return AsnOrderItems
        End Get
    End Property

    Public AsnOrderItems As New List(Of AsnOrderItem)
End Class

Public Class SoOrder
    Inherits BaseOrder

    Public Property ConsigneeAddress() As String
    Public Property ConsigneeContact() As String
    Public Property BillToAddress() As String
    Public Property BillToContact() As String
    Public Property SourceCode() As String
    Public Property ConsigneeName() As String
    Public Overrides ReadOnly Property OrderItems() As IEnumerable(Of BaseOrderItem)
        Get
            Return SoOrderItems
        End Get
    End Property

    Public SoOrderItems As New List(Of SoOrderItem)

End Class

Here is BaseConfirmation class

Public MustInherit Class BaseOrderConfirmation
Private _itransactionService As ITransactionService

Protected Sub AssignNewTransactionsToOrderItems(orders As List(Of BaseOrder))
    For Each order In orders
        For Each item In order.OrderItems
            If (Not item.TransactionId.HasValue) Then
                Dim newTransactionId = _itransactionService.GenerateTransactionId()    
                _itransactionService.AssignTransactionToOrderItem(order.OrderReference, item.Sku, item.Id, newTransactionId)
                item.TransactionId = newTransactionId
            End If
        Next
    Next
End Sub

Protected Sub FilterForConfirmation(orders As IEnumerable(Of BaseOrder))
    For Each order In orders 
        order.OrderItems.RemoveAll(Function(i) _itransactionService.IsTransactionProcessedSuccesfully(order.OrderReference, i.Id))
    Next
End Sub
End Class

The only difference between this code and code suggested above is that OrderItem are located where they should located - in BaseOrder, not in BaseConfirmation class.