1

I've created a very simple helper class that I can use in my ASP.Net pages. The idea is that it is supposed to be a very simple way to log an on-page error or success (not a form validation error) and then display it to the user.

In my public helper class I have a class which has certain properties, as shown below:

public class UserMessage
{
    public UserMessage()
    {
        Messages = new Dictionary<string, string>();
    }

    public string SummaryMessage;
    public Dictionary<string, string> Messages;
    public bool ShowMessages;
    public bool ShowAsError;
}

I then have a variable which is used to store an instance of the UserMessage class, like so:

private static UserMessage _userMessage { get; set; }

I then have two public static methods, one to log a message, the other to display all the messages, like so:

public static void LogSummary(string summaryMessage, bool showIndividualMessages, bool showAsError)
{
    _userMessage = new UserMessage();
    _userMessage.SummaryMessage = summaryMessage;
    _userMessage.ShowMessages = showIndividualMessages;
    _userMessage.ShowAsError = showAsError;
}

public static string DisplayUserMessages()
{
    if (_userMessage == null)
        return string.Empty;

    StringBuilder messageString = new StringBuilder();
    messageString.AppendFormat("\n");
    messageString.AppendLine(string.Format("<div class=\"messageSummary {0}\">", (_userMessage.ShowAsError) ? "invalid" : "valid"));
    messageString.AppendLine(string.Format("<h3>{0}</h3>", _userMessage.SummaryMessage));
    messageString.AppendLine("</div>");

    return messageString.ToString();
}

The problem I have is that the _userMessage variable has to be a static variable, otherwise I get the error message "An object reference is required for the non-static field.......". The problem with the variable being static is that is stays in memory, so if the user receives an error message and then visits another page - the error message is still displayed!

I'm sure this is because I missed OOP 101, but how should I correct this?

2
  • You cannot access nonstatic variable from static methods, that's why it works after you made the _userMessage field static. Apr 25, 2012 at 12:38
  • The bigger problem is that static in ASP.NET means that every request/user will use the same variable. Apr 25, 2012 at 12:39

4 Answers 4

1

Do not use static variable to keep messages per user! ASP.NET application is multi-threaded and using static variable is not thread safe. Store them in Session.

public static void LogSummary(string summaryMessage, ...)
{
   HttpContext.Current.Session["userMessages"] = new UserMessage(); 
   ...
}

public static string DisplayUserMessages()
{
   // get the value from session
   var userMessage = (UserMessage)HttpContext.Current.Session["userMessages"];
   // do the work
   // do the clean up
   HttpContext.Current.Session["userMessages"] = null;
   // the messages will not be displayed on next request
}

Each request is handled by different thread, so the users will overwrite the _userMessage field and you cannot guarantee that messages for the current user will be displayed.

1
  • The Session option worked really well for my solution. Thanks.
    – higgsy
    Apr 26, 2012 at 17:17
1

Pass the reference as an argument to the static member, or have it return a new instance like below:

public static UserMessage LogSummary(string summaryMessage, bool showIndividualMessages, bool showAsError)
{
    var userMessage = new UserMessage();
    userMessage.SummaryMessage = summaryMessage;
    userMessage.ShowMessages = showIndividualMessages;
    userMessage.ShowAsError = showAsError;
    return userMessage;
}
2
  • You're returning instance of a UserMessage class from a void method. Also what will you do with it after? Apr 25, 2012 at 12:40
  • 1
    @KarelFrajtak In this case, I'd probably wrap his DisplayUserMessages up in the UserMessage class, and role the whole thing up in a user control.
    – asawyer
    Apr 25, 2012 at 12:43
0

In my opinion you try to face a problem with a wrong approach. Considering that you're developing service side component (ASP.NET) and you have to have perfect isolation between every user visits your site, I personally don't see any reason why do not use a backend database for error message holding where every record can be associated to a single user unique ID.

The simple ACID supported database (in practice almost any on the market) is a perfect fit in this case.

In this way you can pull from the database the message you need at the moment you need, and don't need more worry about any type of memory issue (at least from the perspective of this question)

Hope this helps.

0

Static variable will be shared within AppDomain - i.e. concurrent requests will share the same instance and hence your approach is problematic.

You should consider putting your user-message instance into the current HttpContext to get per request semantics as needed by your use case. For example,

public class UserMessage
{
   public static UserMessage Current
   {
      get { return HttpContext.Current.Items["_User_Message"] as UserMessage; }
   }

   public static void LogSummary(string summaryMessage, bool showIndividualMessages, bool showAsError)
   {
      var userMessage = new UserMessage();
      userMessage.SummaryMessage = summaryMessage;
      ...  

      HttpContext.Current.Items["_User_Message"] = userMessage;
   }

   public static string DisplayUserMessages()
   {
       var userMessage = UserMessage.Current;
       if (userMessage == null ) return string.Empty;

       ...
   }

   // rest of the code
   ...
}

I would probably also make UserMessage constructor private.

Your Answer

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

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