HiveBrain v1.2.0
Get Started
← Back to all entries
principlecsharpMinor

OOP: Need tips of my approach in .Net

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
tipsneednetapproachoop

Problem

I am looking for some different approaches to this following OOP scenario.

Dim pb As Object = Nothing

 If radCars.Checked = True Then

     Dim productBase As ProductsBase(Of CarColumns)
     productBase = New Cars(fileLocation)

     pb = productBase

  ElseIf radTrucks.Checked = True Then

     Dim productBase As ProductsBase(Of TruckColumns)
     productBase = New Truck(fileLocation)

     pb = productBase

  End If

  pb.Parse()
  pb.AddRows()
  ....


Having to declare the object above and losing intelisense seems off. The base class if using generics because the Truck and Car classes use different column classes for the parsing and adding of columns in the db.

Base Class:

Public MustInherit Class ProductsBase(Of T)

    Sub New(filePathString As String)
        FilePath = filePathString
    End Sub

    MustOverride Sub Parse()
    MustOverride Sub DeleteRows()
    MustOverride Sub AddRows(ByVal rows As IEnumerable(Of T), pageNumber As Integer)

    Property ParseResult As ProductLookupContainer(Of T)

End Class


Car Class:

Public Class Cars
    Inherits ProductsBase(Of CarColumns)

    Sub New(filePath As String)
        MyBase.New(filePath)
    End Sub

    Public Overrides Sub AddRows(rows As IEnumerable(Of CarColumns), pageNumber As Integer)
        DatabaseHelper.AddItemsToDB(Of CarProduct)(ConnectionString, rows, pageNumber)
    End Sub

    Public Overrides Sub DeleteRows()
        DatabaseHelper.DeleteRows(ConnectionString, "CarProducts")
    End Sub

    Public Overrides Sub Parse()

        Dim pf As New ParseFile(FilePath)
        Dim result = New ProductLookupContainer(Of CarColumns)
        result = pf.ParseCars

       'Set object
       ParseResult = result

    End Sub

End Class

Solution

I would agree that defining pb as an object is a code smell and should be eliminated.

Either in addition or instead of having Cars and Trucks inherit from ProductsBase(of T) it should implement an interface.

Suggested Interface:

Public Interface IProducts

    Sub DeleteRows()
    Sub Parse()

End Interface


Then you would declare pb as IProducts. Add any other public properties that will be used by all the derived classes of ProductsBase.

I'd also suggest creating a class or modules that contains factory methods to generate the new objects.

Suggested Factory Class:

Public Class ProductsFactory

    Public Shared Function CreateTruck(fileLocation as string) As IProducts
         Return New Truck(fileLocation)
    End Function

    Public Shared Function CreateCar(fileLocation as string) As IProducts
         Return New Car(fileLocation)
    End Function

End Class


The final code would look like this:

Dim pb As IProducts 

  If radCars.Checked = True Then   
     pb = ProductsFactory.CreateCar(fileLocation)
  ElseIf radTrucks.Checked = True Then
     pb = ProductsFactory.CreateTruck(fileLocation)
  End If

  pb.Parse()
  pb.AddRows()

Code Snippets

Public Interface IProducts

    Sub DeleteRows()
    Sub Parse()

End Interface
Public Class ProductsFactory

    Public Shared Function CreateTruck(fileLocation as string) As IProducts
         Return New Truck(fileLocation)
    End Function

    Public Shared Function CreateCar(fileLocation as string) As IProducts
         Return New Car(fileLocation)
    End Function

End Class
Dim pb As IProducts 

  If radCars.Checked = True Then   
     pb = ProductsFactory.CreateCar(fileLocation)
  ElseIf radTrucks.Checked = True Then
     pb = ProductsFactory.CreateTruck(fileLocation)
  End If

  pb.Parse()
  pb.AddRows()

Context

StackExchange Code Review Q#15489, answer score: 3

Revisions (0)

No revisions yet.