Storing Temporal Message Lists in ASP.NET MVC TempData

Question

I've been using the following the code to store temporal error messages in TempData, to be displayed from my master page on production sites without any problems, but is this a good way of doing it?

The controller helper code:

namespace System.Web.Mvc
{
    public static class Controllers
    {
        public static void SetMessage(this ControllerBase controller, string message)
        {
            List<string> messages = controller.TempData["Messages"] as List<string> ?? new List<string>();
            messages.Add(message);
            controller.TempData["Messages"] = messages;
        }
        public static void SetErrorMessage(this ControllerBase controller, string errorMessage)
        {
            List<string> messages = controller.TempData["ErrorMessages"] as List<string> ?? new List<string>();
            messages.Add(errorMessage);
            controller.TempData["ErrorMessages"] = messages;
        }
    }
}

Example Use:

public ActionResult Edit()
{
    // .. do stuff
    this.SetMessage("You edited X.");
    return View();
}

Here is the rendering code:

<%
    if (TempData["ErrorMessages"] != null)
    {
    %>
        Error Messages
        <%
        foreach (string s in TempData["ErrorMessages"] as List<string>)
        {
            %><%=s%><%
        }
    }

    if (TempData["Messages"] != null)
    {
    %>
        Messages
        <%
        foreach (string s in TempData["Messages"] as List<string>)
        {
            %><%=s%><%
        }
    }
%>

Can you spot any issues?

Answer 1

On the whole the approach is fine, but do not use TempData unless you're looking to populate this information across redirects. TempData comes with a host of issues, and its behaviour isn't quite what you think.

ViewData, on the other hand, is designed exactly for this purpose but comes with the caveat that it doesn't persist across redirects.

Therefore, if you're looking to put something in place which allows developers to push messages into your views easily, you need to expose the ability to put data in either ViewData or TempData, depending on whether they want to redirect or not.

Another option is to clear the TempData entry manually after the call has finished, but I wouldn't recommend this approach.

As a final point, your function names SetMessage and SetErrorMessage are misleading in that they imply you've got one spot only for the message. To me, as a user of your function, when doing this:

this.SetMessage("Foo");
this.SetMessage("Bar");

I would assume that my message is now set to "Bar", but behind the scenes it's a list with both "Foo" and "Bar". I'd recommend changing your function names to AddMessage and AddErrorMessage instead, which implies that behind the scenes there is a collection of elements.