CODEWITHSUNDEEP
vb.net
Biff MaGriff
Biff MaGriff

Reputation: 8231

Is there an elegant way to write this code?

I've inherited some code and it is making me cringe when I look at it. Is there more elegant way to write the following?

Dim myItem As DTO.MyBaseClass = Nothing
Dim myType As String = GetTypeString()
Select Case myType
  Case Is = "Case1"
    myItem = Bus.BusManager(Of DTO.MyClass1).Read()
  Case Is = "Case2"
    myItem = Bus.BusManager(Of DTO.MyClass2).Read()
'... etc etc for 30 lines

Is there a way to make a map from the string to the class type and then just have a line like so? Or something similar?

myItem = Bus.BusManager(Of MappingDealy(myType)).Read()

Upvotes: 5

Views: 274

Answers (3)

CoderDennis
CoderDennis

Reputation: 13837

Since BusManager is a Generic, the type you pass into Of <type> must be specified at compile time. It's not like a traditional parameter that you can change at runtime.

It's unclear from the code you listed what BusManager actually does. If all it's doing is creating an instance of the Generic type, then maybe the person who created it doesn't really understand generics. Do you have the ability to rework how BusManager works, or are you limited to using it as is?

As @jmoreno mentioned, you can use reflection to create an instance of a type from a string containting the name of the type. Here's how that would work: 

Imports System.Reflection
Imports System.IO

Public Class ObjectFactory
    Private Shared Function CreateObjectFromAssembly(ByVal assembly As Assembly, ByVal typeName As String) As Object
        ' resolve the type
        Dim targetType As Type = assembly.GetType(typeName)
        If targetType Is Nothing Then
            Throw New ArgumentException("Can't load type " + typeName)
        End If

        ' get the default constructor and instantiate
        Dim types(-1) As Type
        Dim info As ConstructorInfo = targetType.GetConstructor(types)
        Dim targetObject As Object = info.Invoke(Nothing)
        If targetObject Is Nothing Then
            Throw New ArgumentException("Can't instantiate type " + typeName)
        End If

        Return targetObject
    End Function

    Public Shared Function CreateObject(ByVal typeName As String) As Object
        Return CreateObjectFromAssembly(Assembly.GetExecutingAssembly, typeName)
    End Function

    Public Shared Function CreateObject(ByVal typeName As String, ByVal assemblyFileName As String) As Object
        Dim assemblyFileInfo = New FileInfo(assemblyFileName)
        If assemblyFileInfo.Exists Then
            Return CreateObjectFromAssembly(Reflection.Assembly.LoadFrom(assemblyFileName), typeName)
        Else
            Throw New ArgumentException(assemblyFileName + " cannot be found.")
        End If
    End Function

End Class

In a production app, I'd probably set the return type for all of these methods to my base class or interface. Just make sure you pass in the full typeName including the Namespace.

With that factory class in place, then the elegant version of your code would look something like this:

Dim myItem as DTO.MyBaseClass = ObjectFactory.CreateObject("DTO." & GetTypeString())

Upvotes: 1

Frazell Thomas
Frazell Thomas

Reputation: 6111

Without seeing more of the code I would recommend use an Enumeration on my Case Statement to prevent minor bugs from appearing.

You could then either use a Factory Method to process the data based on the Enumeration.

Upvotes: 0

Ry-
Ry-

Reputation: 224942

First of all, never use Case Is = and never initialize to Nothing. So a quick one would be:

Dim myItem As DTO.MyBaseClass
Select Case GetTypeString()
    Case "Case1"
        myItem = Bus.BusManager(Of DTO.MyClass1).Read()
    ' etc etc

But since you're using templating, there's really no way to map it unless you want to use reflection, which is horribly inefficient at the expense of slightly cleaner and shorter code. You could also add an Imports DTO to save on 124 characters, and also Bus to save on another 120 characters.

Upvotes: 0

Related Questions