5
\$\begingroup\$

This is a class that takes a PropertyInfo and makes a Control for it. You can access the value of the control through a property on the class called Value. Basically I get the data from the database for a specific type and create a form where I add the controls generated by instances of this class for each property of the type. Then, I can get the values back, fill the entity with them and save the changes to the database.

Imports System.Reflection
Imports System.Data.Entity
Imports System.Data.Entity.Core.Common.EntitySql
Imports System.Runtime.Remoting.Messaging
Imports System.IO
Imports System.Linq.Expressions
Imports SCD.Forms
Imports SCD.Helpers
Imports Microsoft.VisualBasic.ApplicationServices
Imports SCD.Models
Imports SCD.Controls
Imports SCD.Services

Namespace AppModels
    Public Interface IViewModelControlMapping
    End Interface

    Public Class ViewModelControlMapping
        Implements IViewModelControlMapping
        Public Property PropertyInfo As PropertyInfo
        Public Property Control As Control
        Private Function GetControl(Of TC As Control)() As TC
            Return DirectCast(Control, TC)
        End Function
        Public Property Value As Object
            Get
                Return GetterFunc()
            End Get
            Set(_value As Object)
                If (SetterFunc IsNot Nothing) Then SetterFunc(_value)
            End Set
        End Property
        Private GetterFunc As Func(Of Object)
        Private SetterFunc As Func(Of Object, Object)

        Public Sub New(propertyInfo As PropertyInfo)
            Me.PropertyInfo = propertyInfo
            If (Not propertyInfo.GetMethod.IsVirtual) Then
                Select Case propertyInfo.PropertyType.Name
                    Case "Byte[]"

                    Case Else
                        Control = New LabelTextBox
                        Control.Dock = DockStyle.Top
                        GetControl(Of LabelTextBox).Label.Text = S(Me.PropertyInfo.Name)
                        GetterFunc = Function() As Object
                                         Return GetControl(Of LabelTextBox).TextBox.Text
                                     End Function
                        SetterFunc = Function(o As Object)
                                         GetControl(Of LabelTextBox).TextBox.Text = o
                                         Return True
                                     End Function
                End Select
            Else
                Dim bindingSource = New BindingSource
                Dim type As Type
                If (propertyInfo.PropertyType.IsGenericType) Then
                    type = propertyInfo.PropertyType.GetGenericArguments()(0)
                    Control = New CrudICollectionControl()
                    Control.Dock = DockStyle.Top
                    Control.SetProperty("LabelText", S(Me.PropertyInfo.Name))
                    GetControl(Of CrudICollectionControl).LabelText = S(Me.PropertyInfo.Name)
                    Dim arr = DatabaseService.GetInstance().GetDbSet(type.Name).ToListAsync().Result.Cast(Of Object).ToArray()
                    GetControl(Of CrudICollectionControl).AdderLabelComboBox.ComboBox.Items.AddRange(arr)
                    AddHandler (GetControl(Of CrudICollectionControl).AdderLabelComboBox.AddRelatedItemButton).Click, Sub()
                                                                                                                          Dim miniCrudFormType = GetType(MiniCrudForm(Of ))
                                                                                                                          Dim typedMiniCrudFormType = miniCrudFormType.MakeGenericType(type)
                                                                                                                          Dim miniCrudForm = Activator.CreateInstance(typedMiniCrudFormType)
                                                                                                                          AddHandler DirectCast(miniCrudForm, Form).Closed, Sub()
                                                                                                                                                                                Dim arr2 = DatabaseService.GetInstance().GetDbSet(type.Name).ToListAsync().Result.Cast(Of Object).ToArray()
                                                                                                                                                                                GetControl(Of CrudICollectionControl).AdderLabelComboBox.ComboBox.Items.Clear()
                                                                                                                                                                                GetControl(Of CrudICollectionControl).AdderLabelComboBox.ComboBox.Items.AddRange(arr2)
                                                                                                                                                                            End Sub
                                                                                                                      End Sub

                    GetterFunc = Function() As Object
                                     Return GetControl(Of CrudICollectionControl).Chips
                                 End Function
                    SetterFunc = Function(o As Object)
                                     If (o IsNot Nothing) Then
                                         GetControl(Of CrudICollectionControl).Chips = o
                                     Else
                                         GetControl(Of CrudICollectionControl).Chips = Nothing
                                     End If
                                     Return True
                                 End Function
                Else
                    Control = New AdderLabelComboBox()
                    type = propertyInfo.PropertyType
                    Dim dbSet = DatabaseService.GetInstance().GetDbSet(type.Name)
                    bindingSource.DataSource = dbSet.ToListAsync().Result
                    GetControl(Of AdderLabelComboBox).Label.Text = S(Me.PropertyInfo.Name)
                    GetControl(Of AdderLabelComboBox).ComboBox.DataSource = bindingSource
                    AddHandler (GetControl(Of AdderLabelComboBox).AddRelatedItemButton).Click, Sub()
                                                                                                   Dim miniCrudFormType = GetType(MiniCrudForm(Of ))
                                                                                                   Dim typedMiniCrudFormType = miniCrudFormType.MakeGenericType(type)
                                                                                                   Dim miniCrudForm = Activator.CreateInstance(typedMiniCrudFormType)
                                                                                                   AddHandler DirectCast(miniCrudForm, Form).Closed, Sub()
                                                                                                                                                         bindingSource.DataSource = dbSet.ToListAsync().Result
                                                                                                                                                         bindingSource.ResetBindings(False)
                                                                                                                                                     End Sub
                                                                                               End Sub
                    GetterFunc = Function() As Object
                                     Return GetControl(Of AdderLabelComboBox).ComboBox.SelectedItem
                                 End Function
                    SetterFunc = Function(o As Object)
                                     GetControl(Of AdderLabelComboBox).ComboBox.SelectedItem = o
                                     Return True
                                 End Function
                End If
                Control.Dock = DockStyle.Top
            End If
        End Sub
    End Class
End Namespace
\$\endgroup\$
  • 1
    \$\begingroup\$ Why do you have an empty interface? \$\endgroup\$ – the_lotus Aug 24 '15 at 15:16
6
\$\begingroup\$
Public Interface IViewModelControlMapping
End Interface

This is called a marker interface - an interface that does nothing. It's a design smell more often than not, because it's misusing interfaces to convey metadata. The framework has attributes for that; when you need metadata for a type, it's best to decorate it with an attribute.

Granted, it's much easier to verify whether a type implements a marker interface, than to use reflection to verify whether it's decorated with such or such attribute... but I'm not going to extrapolate about whether or not you're using the interface as metadata. Instead, I'm going to suggest making it useful!

Property PropertyInfo As PropertyInfo
Property Control As Control
Property Value As Object

The interface should have at least these members; they're public members of any ViewModelControlMapping object, and they're part of that object's interface1 anyway.


Now, where's the code that's doing all the work?

Public Sub New(propertyInfo As PropertyInfo)

The constructor! The constructor is doing all the work! This can't be right. You have quite a lot of logic in there; I'd start refactoring by extracting private methods out of each If/Else block.

The GetControl generic method is interesting:

Private Function GetControl(Of TC As Control)() As TC
    Return DirectCast(Control, TC)
End Function

I'm not too fond of the VB.NET syntax, so for my own sake I'll "translate" it:

private TC GetControl<TC>() where TC : Control
{
    return (TC)Control;
}

Okay. I hope I got this right, the VB syntax for generics is quite foreign to my see-sharp eyes.. Point is, I don't think that method is really needed; all it does is substitute the DirectCast keyword (statement?) for a just-as-convoluted GetControl generic method call: the code would be more self-explanatory if the DirectCast remained at the call sites, so another refactoring I'd do, is inline all these method calls and remove GetControl.

Select Case propertyInfo.PropertyType.Name
    Case "Byte[]"

    Case Else
        Control = New LabelTextBox
        Control.Dock = DockStyle.Top
        GetControl(Of LabelTextBox).Label.Text = S(Me.PropertyInfo.Name)
        GetterFunc = Function() As Object
                         Return GetControl(Of LabelTextBox).TextBox.Text
                     End Function
        SetterFunc = Function(o As Object)
                         GetControl(Of LabelTextBox).TextBox.Text = o
                         Return True
                     End Function
End Select

Why a Select Case with nothing but an Else branch? This really looks like it should be an If block that does nothing if the type is a Byte[] - the question is, why special-case a byte array, and not a Smurf[]?

You're calling GetControl (and so, casting that Control object) 3 times in a row here. Why not declare a LabelTextBox local variable, and code against that instead? If closures work the same way in VB as they do in C#, there shouldn't be a problem with doing that.

I have a bit of a problem with a "Setter" function. Setters don't return true. Setters just set a value, and don't return anything: a setter isn't a Function, it's an Action - an Action delegate that takes one parameter. Don't ask me the VB syntax for it though.


This looks redundant:

 If (o IsNot Nothing) Then
     GetControl(Of CrudICollectionControl).Chips = o
 Else
     GetControl(Of CrudICollectionControl).Chips = Nothing
 End If

If o isn't Nothing, you assign o. Otherwise, you assign Nothing. Why not just assign o and thus, have it be Nothing when o is Nothing?


It looks like the architecture is wrong. There is a different type involved in each conditional branch, and I don't recognize any of them - either my WinForms is much more outdated than I thought, or each branch creates user-defined custom UserControl objects - and with time the list will only grow, and that constructor will get more and more bloated and convoluted.

I don't have an immediate solution for it, but - food for thought - I'd consider implementing a Strategy Pattern here.


1 an object's interface is the public members it exposes - its API, in strict OOP terms; not to be confused with an Interface, which is a type.

| improve this answer | |
\$\endgroup\$

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.