views:

595

answers:

5

This question was inspired by my struggles with ASP.NET MVC, but I think it applies to other situations as well.

Let's say I have an ORM-generated Model and two ViewModels (one for a "details" view and one for an "edit" view):

Model

public class FooModel // ORM generated
{
    public int Id { get; set; }
    public string FirstName { get; set; }
    public string LastName { get; set; }
    public string EmailAddress { get; set; }
    public int Age { get; set; }
    public int CategoryId { get; set; }
}

Display ViewModel

public class FooDisplayViewModel // use for "details" view
{
    [DisplayName("ID Number")]
    public int Id { get; set; }

    [DisplayName("First Name")]
    public string FirstName { get; set; }

    [DisplayName("Last Name")]
    public string LastName { get; set; }

    [DisplayName("Email Address")]
    [DataType("EmailAddress")]
    public string EmailAddress { get; set; }

    public int Age { get; set; }

    [DisplayName("Category")]
    public string CategoryName { get; set; }
}

Edit ViewModel

public class FooEditViewModel // use for "edit" view
{
    [DisplayName("First Name")] // not DRY
    public string FirstName { get; set; }

    [DisplayName("Last Name")] // not DRY
    public string LastName { get; set; }

    [DisplayName("Email Address")] // not DRY
    [DataType("EmailAddress")] // not DRY
    public string EmailAddress { get; set; }

    public int Age { get; set; }

    [DisplayName("Category")] // not DRY
    public SelectList Categories { get; set; }
}

Note that the attributes on the ViewModels are not DRY--a lot of information is repeated. Now imagine this scenario multiplied by 10 or 100, and you can see that it can quickly become quite tedious and error prone to ensure consistency across ViewModels (and therefore across Views).

How can I "DRY up" this code?

Before you answer, "Just put all the attributes on FooModel," I've tried that, but it didn't work because I need to keep my ViewModels "flat". In other words, I can't just compose each ViewModel with a Model--I need my ViewModel to have only the properties (and attributes) that should be consumed by the View, and the View can't burrow into sub-properties to get at the values.

Update

LukLed's answer suggests using inheritance. This definitely reduces the amount of non-DRY code, but it doesn't eliminate it. Note that, in my example above, the DisplayName attribute for the Category property would need to be written twice because the data type of the property is different between the display and edit ViewModels. This isn't going to be a big deal on a small scale, but as the size and complexity of a project scales up (imagine a lot more properties, more attributes per property, more views per model), there is still the potentially for "repeating yourself" a fair amount. Perhaps I'm taking DRY too far here, but I'd still rather have all my "friendly names", data types, validation rules, etc. typed out only once.

+5  A: 

Declare BaseModel, inherit and add another properties:

public class BaseFooViewModel
{
    [DisplayName("First Name")]
    public string FirstName { get; set; }

    [DisplayName("Last Name")]
    public string LastName { get; set; }

    [DisplayName("Email Address")]
    [DataType("EmailAddress")]
    public string EmailAddress { get; set; }
}

public class FooDisplayViewModel : BaseFooViewModel
{
    [DisplayName("ID Number")]
    public int Id { get; set; }
}

public class FooEditViewModel : BaseFooViewModel

EDIT

About categories. Shouldn't edit view model have public string CategoryName { get; set; } and public List<string> Categories { get; set; } instead of SelectList? This way you can place public string CategoryName { get; set; } in base class and keep DRY. Edit view enhances class by adding List<string>.

LukLed
I've played around with using inheritance before. It definitely gets you closer, but it lacks flexibility. One problem is that some attributes still need to be repeated, like the `DisplayName` attribute for `Category` in my example. Obviously, this is not a big deal on a small scale, but on a large scale, you could end up with a lot of double or triple entries of the same `DisplayName` for different ViewModels (and that's just one of several attributes you might want to set for a given property).
DanM
@DanM : I added comment about Category.
LukLed
@LukLed, I'm not sure I'm following you. I'm using `SelectList` because, when the view is scaffolded, a `DropDownList` will be created with the correct list and initially selected item. I can even hook up the `DataValueField` and `DataTextField` for cases where the list is actually a database table. If I just use a `List<string>`, I don't believe my view would be correctly scaffolded.
DanM
@DanM: If you divide SelectList into two properties, you will have to use `Html.DropDownListFor(model => model.Category, new SelectList(Model.Categories, Model.Category))` in your view. Everything will still work.
LukLed
@LukLed, the problem is, I'm trying to use `EditorFor()` and `DisplayFor()` to auto-scaffold the view, so I think I need to actually include the SelectList in my ViewModel. If there's another way, though, I'm all ears.
DanM
@DanM: If you want to use EditorFor then it is not solution, sorry. Maybe metadata class shared by display and edit model could be solution.
LukLed
@LudLed, thanks for your help anyway.
DanM
+5  A: 

I'll assume that your doing this to take advantage of the HtmlHelpers EditorFor and DisplayFor and don't want the overhead of ceremoniously declaring the same thing 4000 times throughout the application.

The easiest way to DRY this up is to implement your own ModelMetadataProvider. The ModelMetadataProvider is what is reading those attributes and presenting them to the template helpers. MVC2 already provides a DataAnnotationsModelMetadataProvider implementation to get things going so inheriting from that makes things really easy.

To get you started here is a simple example that breaks apart camelcased property names into spaces, FirstName => First Name :

public class ConventionModelMetadataProvider : DataAnnotationsModelMetadataProvider
{
    protected override ModelMetadata CreateMetadata(IEnumerable<Attribute> attributes, Type containerType, Func<object> modelAccessor, Type modelType, string propertyName)
    {
        var metadata = base.CreateMetadata(attributes, containerType, modelAccessor, modelType, propertyName);

        HumanizePropertyNamesAsDisplayName(metadata);

        if (metadata.DisplayName.ToUpper() == "ID")
            metadata.DisplayName = "Id Number";

        return metadata;
    }

    private void HumanizePropertyNamesAsDisplayName(ModelMetadata metadata)
    {
        metadata.DisplayName = HumanizeCamel((metadata.DisplayName ?? metadata.PropertyName));
    }

    public static string HumanizeCamel(string camelCasedString)
    {
        if (camelCasedString == null)
            return "";

        StringBuilder sb = new StringBuilder();

        char last = char.MinValue;
        foreach (char c in camelCasedString)
        {
            if (char.IsLower(last) && char.IsUpper(c))
            {
                sb.Append(' ');
            }
            sb.Append(c);
            last = c;
        }
        return sb.ToString();
    }
}

Then all you have to do is register it like adding your own custom ViewEngine or ControllerFactory inside of Global.asax's Application Start:

ModelMetadataProviders.Current = new ConventionModelMetadataProvider();

Now just to show you I'm not cheating this is the view model I'm using to get the same HtmlHelper.*.For experience as your decorated ViewModel:

    public class FooDisplayViewModel // use for "details" view
    {
        public int Id { get; set; }

        public string FirstName { get; set; }

        public string LastName { get; set; }

        [DataType("EmailAddress")]
        public string EmailAddress { get; set; }

        public int Age { get; set; }

        [DisplayName("Category")]
        public string CategoryName { get; set; }
    }
jfar
Thanks, jfar and +1. Yes, exactly, I'm trying to use DisplayFor() and EditorFor() (although, even in cases where I can't, I'd still like to DRY up my ViewModels). Your idea would remove a lot of need for attributes, which is a great help. I wonder, though, if I could also add a custom property (and a parallel custom attribute) that would indicate whether to scaffold a particular property for a particular ViewModel. This would allow me have one ViewModel that handles all views, which means I would never or almost never need to repeat attributes.
DanM
Well the only limitation is the default properties of ModelMetadata. If you need to add more information and create a MyModelMetadata : ModelMetatdata you'll also have create your own custom ViewPage with a custom MyModelMetadata property OR cast the ViewData.ModelMetadata inside whatever .aspx or .ascx files your using.
jfar
+1  A: 

As LukLed said you could create a base class that the View and Edit models derive from, or you could also just derive one view model from the other. In many apps the Edit model is basically the same as View plus some additional stuff (like select lists), so it might make sense to derive the Edit model from the View model.

Or, if you're worried about "class explosion", you could use the same view model for both and pass the additional stuff (like SelectLists) through ViewData. I don't recommend this approach because I think it's confusing to pass some state via the Model and other state via ViewData, but it's an option.

Another option would be to just embrace the separate models. I'm all about keeping logic DRY, but I'm less worried about a few redundant properties in my DTOs (especially on projects using code generation to generate 90% of the view models for me).

Seth Petry-Johnson
+1  A: 

First thing i notice - you got 2 view models. See my answer here for details on this.

Other things that springs in mind are already mentioned (classic approach to apply DRY - inheritance and conventions).


I guess i was too vague. My idea is to create view model per domain model and then - combine them at view models that are per specific view. In your case: =>

public class FooViewModel {
  strange attributes everywhere tralalala
  firstname,lastname,bar,fizz,buzz
}

public class FooDetailsViewModel {
   public FooViewModel Foo {get;set;}
   some additional bull**** if needed
}

public class FooEditViewModel {
   public FooViewModel Foo {get;set;}
   some additional bull**** if needed
}

That allows us to create more complex view models (that are per view) too =>

public class ComplexViewModel {
    public PaginationInfo Pagination {get;set;}
    public FooViewModel Foo {get;set;}
    public BarViewModel Bar {get;set;}
    public HttpContext lol {get;set;}
}

You might find useful this question of mine.

hmm... turns out i actually did suggest to create 3 view models. Anyway, that code snippet kind a reflects my approach.

Another tip - i would go with filter & convention (e.g. by type) based mechanism that fills viewdata with necessary selectList (mvc framework can automagically bind selectList from viewData by name or something).

And another tip - if you use AutoMapper for managing your view model, it has nice feature - it can flatten object graph. Therefore - you can create view model (which is per view) that directly has props of view model (which is per domain model) whatever how much deep you want to go (Haack said it's fine).

Arnis L.
Thanks, Arnis. It sounds like you're saying I should have a *third* view model (e.g., `FooEditPostViewModel`). I have actually thought about doing that at various times, but it only exacerbates the problems I'm having with attributes. I'm intrigued by the Fluent MetadataProvider, though. Will look into that.
DanM
@DanM updated a bit my answer.
Arnis L.
Arnis, thanks for the extra detail. One thing I'm interested in doing is auto-scaffolding (using `DisplayFor()` and `EditorFor()`). Whether I use composition or inheritance, things end up in the wrong order or show up as indented when they shouldn't.
DanM
As for `SelectList`, I think I'm already doing what you suggest. I have a template called SelectList.ascx that automatically creates a `DropDownList`. I have been experimenting with AutoMapper as well, but I don't see how it solves my problem of not repeating my attributes.
DanM
@DanM order is the same as in view model (top => down, i guess). But you might want to set them explicitly - http://blog.maartenballiauw.be/post/2010/01/11/Ordering-fields-in-ASPNET-MVC-2-templated-helpers.aspx
Arnis L.
@DanM with 'filling viewData with selectLists' i mean Filter (applied on actions) that takes entity type as input, through non-generic repository (cause attributes can't and won't be generic) retrieves list of entities, from viewModel, through TypeDescriptor, figures out selected item, forms selectList, adds it to viewData and template that renders whole stuff out as html select tag. I'm not giving any guarantees of this approach, but it seemed quite nice last time i toyed with it.
Arnis L.
@DanM mentioned AutoMapper because it can help to manage nested view models (and more...), not because it can make your code more DRY.
Arnis L.
@DanM and use `@Arnis Lapsa`. That will ensure i'll be noticed and take a look at this question again. :)
Arnis L.
You should receive notification even if I don't use your name because I'm commenting on *your* answer. When you want to get my attention, however, please continue to use @DanM, since I would not receive notification otherwise. See http://blog.stackoverflow.com/2010/01/new-improved-comments-with-reply/ under "Comment @username Notifications".
DanM
@DanM Ah... nevermind. I'm just tired and forgot about that. ;)
Arnis L.
A: 

These display names (the values) could perhaps be displayed in another static class with a lot of const fields. Wouldn't save you having many instances of DisplayNameAttribute but it would make a name change quick and easy to make. Obviously this is isn't helpful for other meta attributes.

If I told my team they would have to create a new model for every little permutation of the same data (and subsequently write automapper definitions for them) they'd revolt and lynch me. I'd rather model metadata that was, too some degree use aware. For example making a properties required attribute only take effect in an "Add" (Model == null) scenario. Particularly as I wouldn't even write two views to handle add/edit. I would have one view to handle the both of them and if I started having different model classes I'd get into trouble with my parent class declaration.. the ...ViewPage bit.

Sam