6
\$\begingroup\$

I read this question and answers, about if Display Annotations on model properties violates the separation of concerns between view and model. And my interpretation of their answer was "No, not if you're using a ViewModel for Display Annotations". But additional separation of concerns questions arose as I began implementing.

I have two questions but both are kind of related to each other. In both, I am wondering if I am violating the separation of concerns:

  1. I'm using localized strings from my views/resources.resx in my Model for Display Attributes, does this violate the separation of concerns since it's a resource in my views folder intended for views?
  2. Do I need a ViewModel for my Model to keep separation of concerns? And if so, which Annotations need to go on my model's properties, and which need to go on the ViewModel properties?

I think I need to keep all the Display Annotations and Validation Annotations (like [required] and [range(x,y)]) that I have in my ViewModel. In my Model I think I need to keep just the Validation Annotations. But, If this is true, then I have two classes to maintain Validation Annotations if I decide to add another property!?

The reason I think that I need Validation Annotations on both classes is so that the Model class can tell EF to put the correct constraints into database when I change schema with DbMigrations. Also, so the ViewModel tells the browser to tell the user when he's putting an invalid value in.

The ViewModel

public class RestaurantReviewViewModel
{
    public int Id { get; set; }
    [Range(1, 10)]
    [Required]
    public int Rating { get; set; }

    [Required]
    [StringLength(1024)]
    [Display(Name = Resources.Description)]
    public string Body { get; set; }

    [Display(Name = Resources.UserName)]
    [DisplayFormat(NullDisplayText = "[Anonymous]")]
    public string ReviewerName { get; set; }
    public int RestaurantId { get; set; }
}

The attributes that I commented out in the next class are the attributes I think i need to remove for the Model.

The Model

public class RestaurantReviewModel
{
    public int Id { get; set; }
    [Range(1, 10)]
    [Required]
    public int Rating { get; set; }

    [Required]
    [StringLength(1024)]
    //[Display(Name = Resources.Description)]
    public string Body { get; set; }

    //[Display(Name = Resources.UserName)]
    //[DisplayFormat(NullDisplayText = "[Anonymous]")]
    public string ReviewerName { get; set; }
    public int RestaurantId { get; set; }
}
\$\endgroup\$
2
\$\begingroup\$

I'm using localized strings from my views/resources.resx in my Model for Display Attributes, does this violate the separation of concerns since it's a resource in my views folder intended for views?

I would probably say yes. You are tying UI concerns in your business models. Why does your model even care about the text that is used to display it? It should be more concerned with buiseness rules and performing buiseness operations I think. Let your UI layer/layers deal with presenting it to the user.

Do I need a ViewModel for my Model to keep separation of concerns? And if so, which Annotations need to go on my model's properties, and which need to go on the ViewModel properties?

No I don't think you necessarilly need a viewmodel and there is plenty of debate on this on the internet. However I personally like the viewmodel approach as it provides the flexibility of adding any UI specified fields I would like without the overhead of my view being tied directly to the buiseness model. Of course this means also that you now have to have the rules copied twice like you mentioned. However if you were using jquery validation this would be the case anyway (or so I believe).

\$\endgroup\$
  • \$\begingroup\$ My UI is using HTML.LabelFor and EditorFor to show names of the model's properties, passing lambda expressions to LabelFor and EditorFor such as (model => model.Name). I know another way is to instead use HTML.Label(Resources.LabelName) instead of 'LabelFor(model => model.Name)` But this changes how the code needs to be maintained by no longer retrieving the name of the property with the lambda expression. Is this the best way if I want the string to be localized? \$\endgroup\$ – Matt Rohde Jun 18 '13 at 20:19
  • \$\begingroup\$ I found a good article to supplement your answer about domain models and inheritance: nohack.eingenetzt.com/system-architecture/… \$\endgroup\$ – Matt Rohde Jul 14 '13 at 22:13
  • 1
    \$\begingroup\$ @MattRohde Cheers matt. Good reading. \$\endgroup\$ – dreza Jul 15 '13 at 0:23
0
\$\begingroup\$

A solution could be:

public class RestaurantReviewViewModel : RestaurantReviewModel
{
    [Display(Name = Resources.Description)]
    public override string Body { get; set; }

    [Display(Name = Resources.UserName)]
    [DisplayFormat(NullDisplayText = "[Anonymous]")]
    public override string ReviewerName { get; set; }
}

public class RestaurantReviewModel
{
    public int Id { get; set; }

    [Range(1, 10)]
    [Required]
    public int Rating { get; set; }

    [Required]
    [StringLength(1024)]
    public virtual string Body { get; set; }

    public virtual string ReviewerName { get; set; }

    public int RestaurantId { get; set; }
}

You can derive from the model class without any problem your attributes will get applied in you view model also (not every but for exmple the validation attributes will). I also recmommend to have copy constructors becouse they can help you a lot.

Beside this my opinion is that no need to worry about display/display format attributes in you model classes it doesn't feel an incorrect way and it helps to keep things clear if you have a lot of derived classes.

\$\endgroup\$
  • 1
    \$\begingroup\$ I understand. There's another error now, The compiler doesn't like this line of code: [Display(Name = Resources.UserName)] and it says "An attribute argument must be a constant expression, typeof expression or array creation expression of an attribute parameter type" So I guess I cannot give localized strings. \$\endgroup\$ – Matt Rohde Jun 16 '13 at 21:53
  • 2
    \$\begingroup\$ Not so sure I'm keen on this. Making a model property virtual so that a viewmodel can override it? \$\endgroup\$ – dreza Jun 16 '13 at 22:38
  • \$\begingroup\$ @Matt: [Display(ResourceType = typeof(Resources), Name = "UserName")] \$\endgroup\$ – Peter Kiss Jun 17 '13 at 5:03
  • \$\begingroup\$ @dreza, why not? \$\endgroup\$ – Peter Kiss Jun 17 '13 at 5:04
  • \$\begingroup\$ Well your viewmodel now inherits all your model to start and is now tightly coupled with it. What happens if you want the properties of the model to do something. The viewmodel not even part of the model domain could prevent that. I guess I'm not 100% exactly sure but it just feels wrong about everything I've read about view models. Of course happy to hear thoughts on it. \$\endgroup\$ – dreza Jun 17 '13 at 7:47

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.