Possible bad practice in Web API ViewModel pattern?

Question

I'm currently implementing ViewModels within my WebApi by injecting a Model object into the constructor of my 'ViewModelProduct' Object, as shown:

public class ViewModelProduct
{
    private IProduct _product;

    private int _ID;
    private string _name;

    public ViewModelProduct(IProduct product)
    {
        _product = product;

        ID = _product.ID;
        Name = _product.Name;
    }

    public int ID
    {
        set { _ID = _product.ID; }
        get { return _ID; }
    }

    public string Name
    {
        set { _name = value; }
        get { return _name;}
    }

    public string Description
    {
        set { _product.Description = value; }
        get { return _product.Description; }
    }

Within the Controller - in my case, the 'ProductController' I want to create an instance of the 'ViewModelProduct'. I want to reduce close coupling where ever possible.

I inject an 'IFacade' object into the constructor of my controller through the use of Unity from a BootStrapper class.

The real question here is I currently have a static method simply returning a new instance of the 'ViewModelProduct' object which I send the Model to to set within its constructor, as shown. Is this bad practice? I can't think how I could create an instance with Unity as I dont know what the model will be before runtime

public List<ViewModelProduct> GetProducts()
{
    var V2Ops = _facade.GetOperatorV2();

    var productList = V2Ops.GetProducts();

    List<ViewModelProduct> listObjects = new List<ViewModelProduct>();

    foreach (var product in productList)
    {
        //*****Setting a new instance from a static method.*****
        var viewModel = CreateNewViewModelV2.CreateViewModel(product);

        listObjects.Add(viewModel);
    }

    return listObjects;
}

Static Class returning a new 'ViewModelProduct' instance:

public static ViewModelProduct CreateViewModel(IProduct passedProductModel)
{
    return new ViewModelProduct(passedProductModel);
}

Answer 1

It's not bad practice, I actually do it all the time but as an extension method (for IProduct). However, in this particular case, do you really need a factory method? Just doing a new ViewModelProduct(product) should be enough.

Your other option though is not quite good. It's a view model, a DTO for an IProduct, using a DI Container is way overkill and has no benefits. Btw, I don't think the viewmodel needs to be abstracted. As data structure it has no real behaviour (at most some helpers), it's not like you'll have multiple variations of it.