Naigel
Naigel

Reputation: 9644

MVC View interacting directly with Model - good or bad?

I have a pretty basic web application showing the content of a table. The user can change some parameters by clicking on links. To simplify let's consider only a simple one, the name of the column used to order rows.

User can either click on the column or select the name of the column from a right sidebar to change the ordering. I use an <option> element to show the list of available column name.

Is it correct to take those names directly from the Model object or should I somehow pass them from the Controller? This is the code by now

<select>
    @{
        // get list of all properties names
        List<string> EtlProceduresNames = (((new MIPortal.Models.EtlProcedure()).GetType().GetProperties()).Select(item => item.Name)).ToList();
        // display an option item for each property name
        foreach (string EtlProceduresName in EtlProceduresNames) { 
            <option value=@EtlProceduresName >@Html.DisplayName(EtlProceduresName)</option>
        }
    }
</select>

This way the View interact directly with Model. Is this a conceptual error? Should I put that code on the Controller and then store the name list on a ViewBag object?

Upvotes: 2

Views: 979

Answers (2)

Mike
Mike

Reputation: 4051

I can suggest you to use so called ViewBag injection. You need to declare an action filter attribute class for your data.

public class EtlProceduresNamesAttribute : ActionFilterAttribute
{
    public override void OnActionExecuting(ActionExecutingContext filterContext)
    {
         //you can add some caching here
         filterContext.Controller.ViewBag.EtlProcedures = (new MIPortal.Models.EtlProcedure()).GetType().GetProperties()).Select(item => item.Name)).ToList();
    } 
}

All you to annotate your view action with this attribute, for example

[EtlProceduresNames]
public ActionResult Action()
{
    return View();
}

You need to replace you view code with

<select>
    @foreach (string EtlProceduresName in ViewBag.EtlProceduresNames) 
    { 
        <option value=@EtlProceduresName >@Html.DisplayName(EtlProceduresName)</option>
    }
</select>

Upvotes: 0

krilovich
krilovich

Reputation: 3505

You should not access the Model directly from the view as that is against the MVC pattern (separation of concern), and you also should NOT be putting this data into the view bag.

View bag should only be used for the simplest of things to pass around and not when you have objects full of data.

Look into using Viewmodels, that is the best way of coding asp.net MVC.

Example of such-

http://www.codeproject.com/Articles/826417/Advantages-of-ViewModel-in-MVC-Model-View-Controll

Upvotes: 4

Related Questions