What can I do better in this ViewModel Creator?

Question

I'm currently creating a ASP.NET MVC page in C#.

I want to hide everything regarding the creation of our "models" and "viewmodels".

I have seen much of the fancy stuff regarding Dependency Injection as well as Action Filters, Custom Controllers and so on! But the people I work with are pretty new to this so I want it to keep it dead simple, but I still want to avoid all the new statement and keep the controller really lightweight.

Please advise me if I'm doing this the wrong way, and be constructive.

Factory Class

 public class Factory
 {
    public T BuildViewModel<T>() where T : class, new()
    {
        IViewModel<T> iVmInterf = (IViewModel<T>)Activator.CreateInstance<T>();

        return iVmInterf.getViewModel();
    }
 }

IViewModel InterFace

 interface IViewModel<T>
 {
    T getViewModel();
 }

interface IProductViewModel

interface IProductViewModel
{
    int ProductCost { get; }
    string ProductName { get; }
}

class ProductViewModel

class ProductViewModel : IViewModel<ProductViewModel>, IProductViewModel
{
    private int _productCost;
    private string _productName;
    private IProductViewModel interf;

    int IProductViewModel.ProductCost
    {
        get { return _productCost; }
    }

    string IProductViewModel.ProductName
    {
        get { return _productName; }
    }

    ProductViewModel IViewModel<ProductViewModel>.getViewModel()
    {
        return this;
    }
}

Usage in controller

Factory fact = new Factory();
IProductViewModel vmdl = fact.BuildViewModel<ProductViewModel>(); 

Answer 1

Var

Use var for method-scope declarations when the right-hand side of the declaration makes the type obvious. This gives you the convenience during refactoring of being able to change the type in just one place.

e.g.

IViewModel<T> iVmInterf = (IViewModel<T>)Activator.CreateInstance<T>();

should be

var iVmInterf = (IViewModel<T>)Activator.CreateInstance<T>();

Naming

You should not include hints about the type of a variable in its name, unless the variable only makes sense with that name. Additionally you should not abbreviate names, abbreviating names makes your code harder to read for no positive benefit.

e.g.

IViewModel<T> iVmInterf

should be

IViewModel<T> viewModel

and

private IProductViewModel interf;

should be

private IProductViewModel viewModel;

Design

Why does ProductViewModel contain an instance variable of type IProductViewModel when it itself implements ProductViewModel? It doesn't appear to be used anywhere and it doesn't make obvious sense why a view model would need a reference to another view model of the same type.

IViewModel<T> takes an instance of itself for T, which I do not agree with from a design perspective. It doesn't make as much sense to say "A view model for product view models" than it does to say "A view model for products". As such I'd recommend making T refer to the model type the view model is for.

Secondly, IViewModel's one method seems entirely useless. Imagine the scenario in which you want to call GetViewModel. Every time you call that method, you will be doing so with an instance of that viewmodel. It's a wasted call, you already have that data.

With that in mind, your factory method becomes one line:

public T BuildViewModel<T>() where T : class, new()
{
    return Activator.CreateInstance<T>();
}