Winform, with a workaround to avoid dependencies

Question

I'm working on a Winform project. We currently have a modular design (shown in the image) in our project. Where on every user action (button clicked or selection changed etc), first we set the Model accordingly (through the controller) then the controller raises an event which is associated with a UI (View) that listens and updates them accordingly.

This flow was designed as such because whenever we need to make the whole logic of the setting/getting data to/from model and controller logic in background, it will be easily done.

For example:

Here I'm uncomfortable with the fact that while listening to an event on the UI, I need to write the code to load all user controls (like I have created a template to load 5 controls). Similarly if there may be other events in which only three controls should be loaded, there I also have to write logic to load controls.

You see, instead of loading every control independently, here I have coupled code.

I think something like dependency property (WPF) will solve this problem. But as Winform Controls are not inheriting DependencyObject I cannot have dependency properties here (at least not as straightforward as WPF) - I have read this.

I have tried the following workaround flow, and my question is, is this approach a proper way to have a workaround for the dependency property?

• Created the ViewModel class, which exposes events for every user controls of UI.
• UI does subscribe those events and updates associated controls in those events.
• Created Model object and pass it to ViewModel, and ViewModel's object is passed in UI.

And the flow will be like:

• When a button will be clicked, UI will ViewModel's the GetUserInfo() method
• ViewModel will call Model's GetUserInfo() method and get actual info
• ViewModel will raise events (to update UI) one by one
• Thus, at the end UI will already be updated, and code behind will not have anything extra to do (just update UI)

Model Class:

public class UserInfoModel
{
    private List<UserInfo> m_userInfoList;

    public UserInfo GetUserInfo(string name)
    {
        //assumption: there will be only one or no user with specified name
        return m_userInfoList.Where(x => x.Name == name).FirstOrDefault();
    }           
}

ViewModel Class:

public delegate void TextBoxUpdateHander(string textBoxText);
public delegate void RadioButtonUpdateHandler(bool radioButtonChecked);
public class UserInfoViewModel
{
    #region Events Associated with UI
    private event TextBoxUpdateHander TxtNameUpdatedEvent;
    public event TextBoxUpdateHander TxtNameUpdatedEventAccessor
    { add { TxtNameUpdatedEvent += value; } remove { TxtNameUpdatedEvent -= value; } }

    private event RadioButtonUpdateHandler RBtnMaleUpdatedEvent;
    public event RadioButtonUpdateHandler RBtnMaleUpdatedEventAccessor
    { add { RBtnMaleUpdatedEvent += value; } remove { RBtnMaleUpdatedEvent -= value; } }
    //**** Same way other events for other UI controls *****//

    #endregion

    UserInfoModel m_userInfoModel;
    public UserInfoViewModel(UserInfoModel userInfoModel)
    {
        this.m_userInfoModel = userInfoModel;
    }

    public void GetUserInfoEventUpdate(string name)
    {
        UserInfo userInfo = this.m_userInfoModel.GetUserInfo(name);
        if (userInfo != null)
        {
            TxtNameUpdate(userInfo.Name);
            RBtnMaleUpdate(userInfo.Gender == Gender.male);
            //**** Same way other methods calling to raise other events *****//
        }
    }

    private void TxtNameUpdate(string name)
    {
        if (TxtNameUpdatedEvent != null)
            TxtNameUpdatedEvent(name);
    }
    private void RBtnMaleUpdate(bool isMale)
    {
        if (RBtnMaleUpdatedEvent != null)
            RBtnMaleUpdatedEvent(isMale);
    }
    //**** Same way other methods to raise other events *****//
}

UI (Form):

public partial class UserInfoView : Form
{
    private UserInfoViewModel m_userInfoViewModel;
    public UserInfoView(UserInfoViewModel userInfoViewModel)
    {
        InitializeComponent();
        m_userInfoViewModel = userInfoViewModel;
        EventSubScriber();
    }

    void m_userInfoViewModel_RBtnMaleAccessor(bool radioButtonChecked)
    {
        rbtnMale.Checked = radioButtonChecked;
    }

    void m_userInfoViewModel_TxtNameAccessor(string textBoxText)
    {
        txtName.Text = textBoxText;
    }
    //***** Same way other event handler methods ****//

    private void btnFindInfoEvent_Click(object sender, EventArgs e)
    {
        m_userInfoViewModel.GetUserInfoEventUpdate(txtNameToFind.Text);
    }

    private void EventSubScriber() {/*subscribing all events*/}

    private void EventUnsubscriber(){//un-subscribing all events}

}

Answer 1

If you start thinking with code, you will almost always end up doing bad things. Why? Because it is not natural. OK, no more preaching and let's get to work.

Review of Your Code

I apologize for being blunt.

1. You are not using C# and .NET coding conventions. You are using Hungarian Notation: m_userInfoViewModel.
2. You are naming your variables with type info in them: m_userInfoList;. A better name would be userInfos. Why? Because it is easier to read and we devs spend more time reading code and less time writing code. So aim for that.
3. What you call a Model is not a Model but a Repository or a Service (not service as in web service) but as in Service Layer.
4. Your ViewModel is aware of the View. This is a big NO.
5. Your ViewModel has TextBoxees (txtName.Text) and radio buttons and click handlers. Again big no! Those belong in the View and the ViewModel should not care whether the data it exposes is displayed in a TextBox or a Label.
6. Your ViewModel has too much unnecessary code.
7. You do not have a Model per-se.

On the other hand, I like the fact that you have a design diagram-big plus from me. Also, you are aiming for Separation of Concerns-another big plus. Good for you :)

Suggestions

Try to think about what you want to accomplish: I have a form wherein I want to allow a user to search users by name and display the found user's information.

That's all you want to do and that can easily be accomplished through databinding.

Let's go through the Model, View, and ViewModel one by one.

Model

Here are a few things from MSDN

The model in the MVVM pattern encapsulates business logic and data.

and

Typically, the model implements the facilities that make it easy to bind to the view. This usually means it supports property and collection changed notification through the INotifyPropertyChanged and INotifyCollectionChanged interfaces.

Most people, especially those who use ORMs such as Entity Framework, confuse this Model with the entity models. They are NOT the same thing. Entity models represent the database tables. A Model in MVVM and MVC is not the same thing as entity models. We programmers, or computer scientists, are horrible at naming things as Phil Karlton has stated really well and discussed here and possibly many other places:

There are only two hard things in Computer Science: cache invalidation and naming things.

For example, UserInfo in your scenario is an entity model. We need to create a Model as in the M in MVVM.

Since we know we will be displaying our users in a view, let's facilitate and make it easy to bind to the view.

public class UserInfoModel : INotifyPropertyChanged
{
    public event PropertyChangedEventHandler PropertyChanged;

    public string Name
    {
        get
        {
            return this.Name;
        }

        set
        {
            if (value != this.Name)
            {
                this.Name = value;
                NotifyPropertyChanged();
            }
        }
    }

    // This method is called by the Set accessor of each property.
    // The CallerMemberName attribute that is applied to the optional propertyName
    // parameter causes the property name of the caller to be substituted as an argument.
    private void NotifyPropertyChanged([CallerMemberName] string propertyName = "")
    {
        if (PropertyChanged != null)
        {
            PropertyChanged(this, new PropertyChangedEventArgs(propertyName));
        }
    }
}

Notice a few things in the above Model:

1. It implements INotifyPropertyChanged interface. Why? Because anytime one of the properties in the Model changes, we will raise an event and inform all subscribers that the property has changed. The subscribers, in your case the controls in your view, will update themselves.
2. I have only done the implementation for one property for brevity.

View

You need to bind to the ViewModel. There is design time support for this and I am not going to go into details about how to do that. Please follow this or this winform tutorial. Even though the first one is about DevExpress controls, the native winform controls have similar but limited options. Actually you may find the entire article helpful.

I will however show how to wire-up your view.

public class UserInfoView : Form
{
    private readonly UserInfoViewModel viewModel;

    public UserInfoView()
    {
        this.viewModel = new UserInfoViewModel(new UserInfoModel());
        InitializeComponent();
    }

    private void FindUserButton_Click(object sender, System.EventArgs e)
    {
        this.viewModel.Find();
    }
}

Notice a few things in the above View:

1. In its constructor, it sets-up a new ViewModel that it will be interacting with.
2. The Find button click handler simply calls this.viewModel.Find(). Does nothing else. The UI controls will automatically refresh due to INotifyPropertyChanged in the model. But keep in mind you will need to bind the controls to the properties either at design time or programmatically.

Since it is your view that will decide who it is interacting with, this is also a good spot to decide on where the data will come from: the repository. More about repository later on.

ViewModel

You do not need almost all the code in your ViewModel except for this:

public class UserInfoViewModel
{
    private UserInfoRepository userInfoRepository;
    public UserInfoViewModel(UserInfoModel userInfoModel)
    {
        this.Model = userInfoModel;
    }

    public UserInfoModel Model { get; set; }

    public string NameToSearch { get; set; }

    public void Find()
    {
        UserInfo userInfo = this.userInfoRepository.Get(this.UserName);
        this.Model = new UserInfoModel();
        if (userInfo != null)
        {
            this.Model.Name = userInfo.Name;
            this.userInfoRepository = new UserInfoRepository();
        }
    }
}

Notice a few things in the above ViewModel:

1. It has a UserInfoRepository and the variable is named userInfoRepository and not m_userInfoRepository because in C# and .NET the hungarian notation is not the convention.
2. The ViewModel uses the repository and asks the repository if a user exists.

3. There is a property named Model and not UserInfoModel. I always do this as a convention because it fits the MVVM and stands for M. The ViewModel is exposing a Model. A ViewModel does not need to always expose the model but in simple cases, it is fine as stated in MSDN:

   The view model may choose to expose model classes directly to the view so that controls in the view can data bind directly to them. In this case, the model classes will need to be designed to support data binding and the relevant change notification events.

   You may be wondering: What if my ViewModel has many models? We will come to this later.

4. There is a property named NameToSearch. This is needed so we can search for users by a name. This property will be bound to to the TextBox used for searching.

5. There is a Find method. This method will be called by the Find button handler. It calls the repository and if a user is found, it populates the model.

But what if my ViewModel has many Models?

The pattern says this:

Typically, there is a one-to many-relationship between the view model and the model classes.

Yes, this is true. In cases wherein I have multiple Models, I do this:

public class XxxModel // Where Xxx is the name of the View
{
    public SomeType SomeModel { get; set; }
    public SomeOtherType SomeOtherModel { get; set; } 
}

Then my ViewModel will still expose a single property named Model:

public class XxxViewModel // Where Xxx is the name of the View
{
    public XxxModel Model { get; set; }
}

This makes the design easier to understand in my opinion. If you want to take this approach, go ahead. If not, no worries.

Few Other Points

1. Some may say that the ViewModel should not be talking to a concrete repository but an interface instead. That is true and good advice. But for starters, since you are learning, you can just do it the way I have shown.

2. The UserInfoModel can be changed so it wraps a UserInfo and then updating that underneath. That is also a possibility to consider. Like this:

   public class UserInfoModel : INotifyPropertyChanged
   {
       private UserInfo userInfo;
       public event PropertyChangedEventHandler PropertyChanged;
   
       public UserInfoModel(UserInfo userInfo)
       {
           this.userInfo = userInfo;
       }
   
       public string Name
       {
           get
           {
               return this.userInfo.Name;
           }
   
           set
           {
               if (value != this.userInfo.Name)
               {
                   this.userInfo.Name = value;
                   NotifyPropertyChanged();
               }
           }
       }
   }