1
\$\begingroup\$

My page contains: GridView1, GridView2, Button1, Button2, DropDownList1 I bind Gridviews to the table selected in dropdown like this:

Dim results as DataTable
Select Case ddl1.SelectedValue
    Case 0
        Dim cl as ClassZero = new ClassZero()
        results = cl.GetClassZeroNames()
    Case 1
           Dim cl as ClassOne = new ClassOne()
        results = cl.GetClassOneNames()
    Case 2
         Dim cl as ClassTwo = new ClassTwo()
        results = cl.GetClassTwoNames()
End Select
GridView1.DataSource = results
GridView1.DataBind()

Then I have two buttons with the following code:

Protected Sub btn1_Click(sender As Object, e As EventArgs) Handles btn2.Click
    Select Case ddl1.SelectedValue
        Case 0
             Dim cl as ClassZero = new ClassZero()
        results = cl.RunInsert()
        Case 1
             Dim cl as ClassOne = new ClassOne()
        results = cl.RunInsert()
        Case 2
             Dim cl as ClassTwo = new ClassTwo()
        results = cl.RunInsert()
    End Select
End Sub

And

Protected Sub btn2_Click(sender As Object, e As EventArgs) Handles btn2.Click
    Select Case ddl1.SelectedValue
        Case 0
             Dim cl as ClassZero = new ClassZero()
        results = cl.RemoveFromZero()
        Case 1
            Dim cl as ClassOne = new ClassOne()
        results = cl.RemoveFromOne()
        Case 2
            Dim cl as ClassTwo = new ClassTwo()
        results = cl.RemoveFromTwo()
    End Select
End Sub

For me it looks like a lot of overhead. How can I improve then design and only specify that I'm working on the following record in dropdown list without specifying the Case condition every time? Should I change my design or leave it like it is?

RunZero, RunOne, RunTwo, RemoveZero, RemoveOne, RemoveTwo, RemoveThree - Execute six different stored procedures.

\$\endgroup\$
  • \$\begingroup\$ I don't think there is much different you can do here. \$\endgroup\$ – shashi Aug 17 '11 at 19:55
  • \$\begingroup\$ Is the title accurate? Multi-threading is involved? If so, that is important but you don't explain any of that in your question ... \$\endgroup\$ – IAbstract Aug 17 '11 at 23:16
2
\$\begingroup\$

You could use delegates. Set delegates in the Change event of ddl1, then you can always call the same delegates in your button handlers. This way you only need a switch in ddl1 Change event, and your button handlers will become much cleaner. However, even though this would make the code much more maintainable/readable, it'd probably be less efficient, though you still wouldn't be able to feel the difference in UI handlers.

Update: example added

Something along these lines (not copy&paste):

Public Delegate Function CreateClassDelegate() As NumberClass

Public Class Form
    Public CreateClass As CreateClassDelegate

    Private Sub ddl1_SelectedValueChanged(sender As Object, e As EventArgs)
        Select Case Integer.Parse(TryCast(ddl1.SelectedItem, String))
            Case 0
                CreateClass = New CreateClassDelegate(AddressOf ClassZero.CreateClassZero)
                Exit Select
            Case 1
                CreateClass = new CreateClassDelegate(AddressOf ClassOne.CreateClassOne);
                Exit Select
            Case 2
                CreateClass = new CreateClassDelegate(AddressOf ClassTwo.CreateClassTwo);
                Exit Select
        End Select
    End Sub

    Private Sub btn1_Click(sender As Object, e As EventArgs)
        Dim cl As NumberClass = CreateClass()
        cl.GetNames()
    End Sub

    Private Sub btn2_Click(sender As Object, e As EventArgs)
        Dim cl As NumberClass = CreateClass()
        cl.RunInsert()
    End Sub
End Class

Public MustInherit Class NumberClass
    Public MustOverride Function GetNames() As List(Of String)
    Public MustOverride Sub RunInsert()
    Public MustOverride Sub Remove()
End Class

Public Class ClassZero
    Inherits NumberClass
    Public Overrides Function GetNames() As List(Of String)
         ...
        Return Nothing
    End Function
    Public Overrides Sub RunInsert()
             ...
    End Sub
    Public Overrides Sub Remove()
         ...
    End Sub

    Public Shared Function CreateClassZero() As ClassZero
        Return New ClassZero()
    End Function
End Class

' ... same as above for ClassOne, ClassTwo
\$\endgroup\$
0
\$\begingroup\$

I like to use Dictionaries for stuff like this. So in your first instance you could create a Dictionary<int,DataTable> and use that instead of the switch statement.

\$\endgroup\$
0
\$\begingroup\$

The switch-statement has very little overhead. A Dictionary will clean it up but isn't as expressive as a switch statement. You still have to instance the dictionary and fill it with the values.

I really don't see anything wrong with what your are doing. Some may nit-pick and recommend that the switch statement should be in another method. When you get down to it, these switch statements are probably the least of your concerns. Save those for when you really have nothing else to work on.

Update
Seeing the update, the first thing I see is the same classes instantiated over and over ...and a couple of ways of dealing with this.

Option 1: Do ClassZero, ClassOne, ClassTwo require a new instance every time? In other words, why not make them static. You won't have the instantiation code repeated. That would clean things up tremendously.

Option 2: If the classes must be instantiated every time, I would think about implementing 2 patterns: Factory & Command

...There are plenty of examples on the internet and going into the design specifics.

Using both of these patterns will move implementation from your UI logic to a business centric layer. For instance, I believe you could achieve something looking like this:

Dim cl as IDataClass = DataClassFactory.CreateDataClass(ddl1.SelectedValue);

Dim results as DataTable = cl.GetClassNames()

GridView1.DataSource = results
GridView1.DataBind()

Basically, then, in each of your button click events, you will create an IDataClass derived ClassZero, ClassOne, ClassTwo.

IDataClass defines the method GetClassNames() which the sub-classes will implement independent of each other.

\$\endgroup\$
  • \$\begingroup\$ I can't agree that switch-statement has little overhead. In this particular example, when dll1 gets new value (4) we'll have to change code in 3 places. So, is think it's better to use Dictionary. It might be not so expressive as switch-statement, but it will allow you to change the code in one place only. \$\endgroup\$ – Ivan Aug 17 '11 at 21:24
  • \$\begingroup\$ Or you can change code in 3 dictionaries. Not much different. A different pattern, possibly the Command pattern would make this much better. But the OP hasn't provided enough code to be able to do much more. \$\endgroup\$ – IAbstract Aug 17 '11 at 21:39
  • \$\begingroup\$ Actually, by Dictionary I meant a class with delegates for every type of event handlers. So in event handler 'btn1_Click' you use it like _dllHandlers[ddl1.SelectedValue].RunInsert(). And for 'btn2_Click' event handler like _dllHandlers[ddl1.SelectedValue].RunRemove(). It also could be implemented as List of such classes, but it's minor details of the implementation. I could write a complete example, if this description is not clear. \$\endgroup\$ – Ivan Aug 18 '11 at 5:59

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.