4
votes

I have certain situations where I need to pass a value between controller actions.

  1. When passing a returnUrl from a view to all nested views. In the view I have

    @{
       TempData["returnURL"] = Request.Url.AbsoluteUri;
    }
    

    and then access it in a similar way to this (in my real version I check that the key is in TempData and that the returnURL is a real URL):

    return Redirect(TempData["returnURL"].ToString());
    

    If it needs to continue on past the first page change (i.e. Search page -> Edit page -> Edit Section page) I'm adding it again

    TempData["returnURL"] = TempData["returnURL"];
    
  2. When I need to pass a value from one controller action through a view to another controller action that is called by ajax such as here:

    public ViewResult Index(FormCollection form)
    {
       var model = new GridColumnChooserViewModel();
    
       //Select deleted/not deleted rows
       if (form.HasKeys())
           model.ShowRows = (form["deletedDropDown"] == null) ? 
                                                     "Active" :
                                                      GetOptionByName(form["deletedDropDown"]);
    
       TempData["ShowRows"] = model.ShowRows;
       ...
    }
    

    and then in my other ajax-called action controller I access it:

    public JsonResult GetData()
    {
       //Select deleted/not deleted rows
       var showRows = (TempData.ContainsKey("ShowRows") && TempData["ShowRows"] == null) ?
                                               "Active" :
                                               GetOptionByName(TempData["ShowRows"].ToString());
    
       //refresh tempdata showrows so it is there for next call
       TempData["ShowRows"] = model.ShowRows;
    
       return this.GetDataSource(showRows);
    }
    

My question is, is this really bad practice? From my understanding of it, I'm essentially using TempData like a session cookie. Is there a better way to do this, like using an actual cookie?

3
FYI - FormCollection is deprecated. - Daniel A. White
Also its a bad idea for a view to set anything. - Daniel A. White
Where does it say FormCollection is deprecated? No mention of that on MSDN: msdn.microsoft.com/en-us/library/…. - dnatoli
Its a better practice to use strongly typed models. - Daniel A. White

3 Answers

1
votes

Yes, I would say that this in general is bad practice. While the ViewData dictionary approach is fast and fairly easy to implement it can leads to typo's and errors that are not caught at compile time. An alternative would be to use the ViewModel pattern which allows you to use strongly-typed classes for the specific view you need to expose values or content within. Ultimately giving you type safe and compile time checking along with intellisense.

My first choice would be to use a view model. If that doesn't fit then using session state may be just fine.

1
votes

It seems like you are using TempData to flow state through the various pages of your site; in general, I'd say this is a bad practice.

Ideally, you would flow whatever upcoming state you would need to the client, and the client would store it (in some sort of JSON or whatever). Then, the client would return it to you as part of their action, and then you'd pass back the appropriate state, etc.; it speaks more to the stateless nature of HTTP applications.

0
votes

I changed both situations to use Session so that I didn't have to keep pushing the TempData value on.

public ActionResult Create()
{
    Session["returnURL"] = Request.UrlReferrer.AbsoluteUri;
    ...
}

Then I access it like this

var returnURL = (Session["returnURL"] != null) ? Session["returnURL"].ToString() 
                                               : Url.Action("Index", "Home");

Seems a bit better.