I wanted to compare with best practices when working with an ORM or database tables in asp.net mvc. One of the major questions I have is should I instantiate the model classes directly in controller..not query the database but just use the model class to store the values.
For e.g. If I am using entity framework as model...then is it a bad practice to use the entity class objects in the controller. There are times when it is just easier to directly use the database classes generated in the controller instead of creating ViewModels or even ViewData. We have a data access layer and a Business layer where all the querying and business logic is applied but although easier I don't like the idea of accessing the model in the controller but is it really a bad practice?
Yes, it is a bad practice, because of the problem of "over-posting".
For instance, consider an Entity model for a UserProfile:
public class UserProfile
{
public string UserName { get; set; }
public bool IsAdmin { get; set; }
public string EmailAddress { get; set; }
public string FirstName { get; set; }
public string LastName { get; set; }
}
Your user profile page allows the user to Edit their FirstName, LastName, and EmailAddress.
An unscrupulous user could simply modify the form to post "IsAdmin" along with the other values. Because your Action is expecting an input of UserProfile, the IsAdmin value will be mapped as well, and eventually persisted.
Here is an excellent writeup about the perils of under and overposting.
I see nothing wrong with binding Entity models directly to your [HttpGet] methods, though.
Actually, it depends, maybe all you need is exactly show your model in UI. then I see no sense in wrapping it. But most of the times, even if you think you do not need to change anything in a model before showing it into view, you may need to do it in future. So the better way is to separate your exact model and future view data. It gives you more flexibility if you ned to change something (like change DB structure but view will remain the same)
Yes, by practice. You should keep controller as skinny as possible and separate each concerns.
You can still use view specific models to handle over posting issues, however there's a newer Bind attribute you can use to achieve the same thing, as outlined in this blog post.
Take Peter J's Entity model for example. On an edit method you could simply do this:
[HttpPost]
public ViewResult Edit([Bind(Include = "UserName, EmailAddress, FirstName, LastName")] User user)
{
// ...
}
And simply leave out the IsAdmin item to prevent it from being used in the post.
Further, as noted in the blog post, you can take a "blacklist" method and tell your controller action which fields to exclude instead:
[HttpPost]
public ViewResult Edit([Bind(Exclude= "IsAdmin")] User user)
{
// ...
}
You can also place it above the class name of the Entity model, like so:
[Bind(Exclude="IsAdmin")]
public class UserProfile
{
public string UserName { get; set; }
public bool IsAdmin { get; set; }
public string EmailAddress { get; set; }
public string FirstName { get; set; }
public string LastName { get; set; }
}
This method may be better suited for when you only need to exclude or include a few fields from your Entity models.