2
\$\begingroup\$

I am walking through a sample MVC4 ASP.NET tutorial available on PluralSight.com, by Scott Allen. I am in a section #5, Working with Data (part 2), "Listing Reviews".

This application has a database of restaurant reviews. I'm following a section right now in which Scott shows how to add links in the Restaurant view that will display a list of reviews for the selected restaurant.

There was a problem when clicking the link to display the reviews, encountered in the following cshtml file. The problem was that Model.Reviews in the parameters for Html.Partial method was null by default.

Index.cshtml

@model OdeToFood.Models.Restaurant

@{
    ViewBag.Title = "Index";
}

<h2>Reviews for @Model.Name</h2>

@Html.Partial("_reviews", Model.Reviews) 

<p>
    @Html.ActionLink("Create New", "Create")
</p>

Scott explained that the Model class Restaurant, listed below, will load all the properties except for Reviews, which are stored in another table somewhere else. He said there are many ways to fix the problem, but the easiest way to make EF load the reviews is to make the Reviews property virtual.

Model class Restaurant

public class Restaurant
{
    public int Id { get; set; }
    public string Name { get; set; }
    public string City { get; set; }
    public string Country { get; set; }
    public ICollection<RestaurantReview> Reviews { get; set; } 
}

He said that at run time, the EF creates a "wrapper" for the restaurant class that will intercept calls (I don't understand this) to the Reviews property so that when you get to the reviews in the view, The EF will load them up instead of being null.

My concern is that making a property virtual when you don't intend to override it, is bad coding practice. If so, what is the best coding practice here? I'm also wondering if the simplest solution is the best solution here.

\$\endgroup\$
3
  • \$\begingroup\$ I'm not sure if your question fits here, because you're not actually asking for a review of your code. \$\endgroup\$
    – svick
    May 22, 2013 at 18:28
  • \$\begingroup\$ @svick Yes, I was trying to decide that. I felt it might be relevant to Code Review as opposed to StackOverflow because I felt that the virtual property needed to be reviewed. \$\endgroup\$
    – Matt Rohde
    May 22, 2013 at 18:37
  • 1
    \$\begingroup\$ making a property virtual when you don't intend to override it, is bad coding practice -- but it is being overridden by generated code. \$\endgroup\$
    – psaxton
    Jun 30, 2015 at 3:49

2 Answers 2

5
\$\begingroup\$

EF will not create a wrapper around the Restaurant class! Instead it will derive from your Restaurant class and it will override that property (public ICollection<RestaurantReview> Reviews) because it needs to create a proxy or navigation property instead of an everyday property. If you don't specify that property as a virtual one, the EF can not create a navigation property over it and your relations will not work.

I recommend you to check out this behavior through the debugger you will see classes with this kind of name: Restaurant_HFUEOHUEOHHRRHRHRHHRHIFEFNSES355KL4KLH4KH3KLNEN3L3

\$\endgroup\$
1
\$\begingroup\$

I considered submitting this for migration to StackOverflow, but mentally justified it as being a meta-review of your own review. Thus I will take this as on opportunity to respond to your review.

I'm not sure if leaving methods virtual is bad coding practice -- Java designers would take the opposite view.

Previous comment aside, you are intending for the property to be overridden. To explain most simply would be to define the 'wrapper' or 'decorator' pattern which you indicated you didn't understand.

Most basically, the pattern involves subclassing a type to add functionality, such as logging, authorization, or in this case creating lazy navigable properties. If EF automatically populated each complex property of an entity (and those properties complex properties ad infinitum), then most common object graphs would result in returning the entire repository. To avoid this, EF creates lazy properties to not load these details until they are first read. (This can be overridden by specifying a property as 'eager', but that is beyond this review.) EF generates a subclass of your entity to 'decorate' this laziness on your type which, in turn requires the property to be virtual.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

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