ASP.NET MVC view model best practices

Question

My ASP.NET MVC site connects to a WCF service to get data. The WCF service returns a data contract like this:

[DataContract]
public class Person
{
    [DataMember]
    public string First { get; set; }

    [DataMember]
    public string Last { get; set; }
}

The view model in my MVC project looks like this:

public class MyViewModel
{
    public string SomeExtraField1 { get; set; }
    public string SomeExtraField2 { get; set; }
    public string SomeExtraField3 { get; set; }

    public Person Person { set; set; }
}

Should my view model be referencing the "Person" data contract that is returned from the data service? Or should I create a new "Person" class in my MVC project that mirrors the properties on the "Person" data contract?

The WCF service call is hidden behind an interface. It seems to be that having the interface reference the data contract makes my interface a leaky abstraction. However, I have a few people that believe creating an additional "Person" class in my MVC project that mirrors the data contract is code bloat.

What are are the best practices surrounding this type of layering/decoupling?

Answer 1

Should my view model be referencing the "Person" data contract that is returned from the data service?

No, avoid this, it's giving developers the false impression that they are using view models. I quite often see code like this when doing code reviews:

public class MyViewModel
{
    public SomeDomainModel1 Model1 { get; set; }
    public SomeDomainModel2 Model2 { get; set; }
    ...
}

and that's just wrong. When I critique them for not using view models they show me this and tell me: "Darin, look, I am using view models", unfortunately that's not how view models are supposed to work. They are not wrappers around domain models.

Or should I create a new "Person" class in my MVC project that mirrors the properties on the "Person" data contract?

Yes, you could create a PersonViewModel and include only the properties that your view needs of course.

Or if the particular view you are designing this view model for needs only some properties you could also make it look like this:

public class MyViewModel
{
    public string SomeExtraField1 { get; set; }
    public string SomeExtraField2 { get; set; }
    public string SomeExtraField3 { get; set; }

    // this would be for example the concatenation of your domain model 
    // first name and last name as that's what this particular view needs to 
    // display
    public string PersonFullName { set; set; }
}

As far as the conversion between your domain models and view models is concerned, AutoMapper is simply put: excellent.