# Generating controls based on PropertyInfo

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()
Dim miniCrudFormType = GetType(MiniCrudForm(Of ))
Dim typedMiniCrudFormType = miniCrudFormType.MakeGenericType(type)
Dim miniCrudForm = Activator.CreateInstance(typedMiniCrudFormType)
Dim arr2 = DatabaseService.GetInstance().GetDbSet(type.Name).ToListAsync().Result.Cast(Of Object).ToArray()
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
type = propertyInfo.PropertyType
Dim dbSet = DatabaseService.GetInstance().GetDbSet(type.Name)
bindingSource.DataSource = dbSet.ToListAsync().Result
Dim miniCrudFormType = GetType(MiniCrudForm(Of ))
Dim typedMiniCrudFormType = miniCrudFormType.MakeGenericType(type)
Dim miniCrudForm = Activator.CreateInstance(typedMiniCrudFormType)
bindingSource.DataSource = dbSet.ToListAsync().Result
bindingSource.ResetBindings(False)
End Sub
End Sub
GetterFunc = Function() As Object
End Function
SetterFunc = Function(o As Object)
Return True
End Function
End If
Control.Dock = DockStyle.Top
End If
End Sub
End Class
End Namespace

• Why do you have an empty interface? – the_lotus Aug 24 '15 at 15:16

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.