3
\$\begingroup\$

I've been working in C# for a while, but I'm just recently starting to force myself to use better abstracted and generic code. I recently encountered a situation where I was juggling a lot of session values on a page, so I made a wrapper class for the session to enable easy and safe get/sets of session values.

I'm not entirely sure if this is the best way to go about accomplishing what I want, but I would love some feedback on ways this code can be improved. I'm aware that it can currently only be used for string values, but I'm just using it right now for storing form values between multiple postbacks in a very complex page, so that's not a huge concern for me at the moment.

Base class

abstract public class SessionManager
{
    public List<string> PossibleKeys;

    public string GetValue(HttpSessionState pSession, string pKey)
    {
        if (this.PossibleKeys.Contains(pKey))
            return ((SessionManager)pSession[this.SessionKey]).Values[pKey].ToString();
        else
            return "";
    }

    public bool SetValue(HttpSessionState pSession, string pKey, string pValue)
    {
        if (this.PossibleKeys.Contains(pKey))
        {
            var tParameters = (SessionManager)pSession[this.SessionKey];
            tParameters.Values[pKey] = pValue.Trim();
            pSession[this.SessionKey] = tParameters;
            return true;
        }
        return false;
    }

    abstract class Keys { }

    private Dictionary<string, string> Values { get; set; }
    private string SessionKey { get; set; }
    private static string DefaultKey = "parameters";

    protected void Construct(HttpSessionState pSession, string pSessionKey = null)
    {
        if (pSessionKey.IsNullOrWhitespace())
            this.SessionKey = SessionManager.DefaultKey;
        else
            this.SessionKey = pSessionKey.Trim();

        this.Values = new Dictionary<string, string>();
        foreach (var tKey in this.PossibleKeys)
        {
            this.Values.Add(tKey, "");
        }

        SessionManager.Save(pSession, this, this.SessionKey);
    }

    private static void Save(HttpSessionState pSession, SessionManager pSelf, string pSessionKey)
    {
        pSession[pSessionKey] = pSelf;
    }        
}

Implemented class on the page codebehind

public class DataManager : SessionManager
{
    public class Keys
    {
        public static string MemberID = "MemberID";
        public static string ProviderID = "ProviderID";
        public static string ReferredToID = "ReferredToID";
        public static string SpecialtyID = "SpecialtyID";
        public static string DiagnosisID = "DiagnosisID";
        public static string CPTServiceID = "CPTServiceID";
        public static string FromDate = "FromDate";
        public static string ToDate = "ToDate";
    }

    public DataManager(HttpSessionState pSession, string pSessionKey = null)
    {
        this.PossibleKeys = new List<string>()
        {
              DataManager.Keys.MemberID
            , DataManager.Keys.ProviderID
            , DataManager.Keys.ReferredToID
            , DataManager.Keys.SpecialtyID
            , DataManager.Keys.DiagnosisID
            , DataManager.Keys.CPTServiceID
            , DataManager.Keys.FromDate
            , DataManager.Keys.ToDate
        };

        base.Construct(pSession, pSessionKey);
    }
}

Actual use in the page codebehind

    public DataManager Parameters { get; set; }
    protected void Page_Load(object sender, EventArgs e)
    {

        if (!IsPostBack)
        {
            this.Parameters = new DataManager(this.Session);
        }

        this.Parameters.SetValue(this.Session, DataManager.Keys.DiagnosisID, "Blah blah, your value goes here");

        string test = this.Parameters.GetValue(this.Session, DataManager.Keys.DiagnosisID);
    }
\$\endgroup\$
3
\$\begingroup\$

I don't get what your solution brings that something like this doesn't:

public class DataManager
{
    private HttpSessionState session
    public String MemberID
    {
        get {return session["MemberID"];}
        set {session["MemberID"] = value;}
    }
}

Managing the collections Keys and PossibleKeys in two places seems error-prone. Your naming convention of prefixing everything with 'p' is unusual (in C#).

\$\endgroup\$
  • \$\begingroup\$ I needed a consistent pattern for safely handling session values in my codebehind files. I've been rewriting a considerable amount of legacy code for my project, and problems around typos with the session values stored are a cause of considerable headaches and bugs. You'll notice that with my system, the only hard-coded strings for the session are defined once. Because I wanted to use this system for multiple pages in the future, I created the flexible base class that I can use on any page, then define the page specific values as needed for that page. \$\endgroup\$ – Kenneth Posey Sep 5 '14 at 20:00
3
\$\begingroup\$

Inside your abstract class you could shorten this part a little bit by being consistent

public string GetValue(HttpSessionState pSession, string pKey)
{
    if (this.PossibleKeys.Contains(pKey))
        return ((SessionManager)pSession[this.SessionKey]).Values[pKey].ToString();
    else
        return "";
}

public bool SetValue(HttpSessionState pSession, string pKey, string pValue)
{
    if (this.PossibleKeys.Contains(pKey))
    {
        var tParameters = (SessionManager)pSession[this.SessionKey];
        tParameters.Values[pKey] = pValue.Trim();
        pSession[this.SessionKey] = tParameters;
        return true;
    }
    return false;
}

instead of using an else statement you could write it like this

public string GetValue(HttpSessionState pSession, string pKey)
{
    if (this.PossibleKeys.Contains(pKey))
    {
        return ((SessionManager)pSession[this.SessionKey]).Values[pKey].ToString();
    }
    return "";
}

Notice my use of braces/brackets. you should always use them, it's a good habit to fall into.

We already know that if you hit that if statement you are going to exit the method through the return statement, so you can say that if the if statement isn't triggered we want to return "";


what is pKey and how does it differ from tKey?

I can see that all the variables preceded with a 'p' are parameters, but I am not following what 't' corresponds to yet?

\$\endgroup\$
  • \$\begingroup\$ Good catch on the option for modifying the if/else statement. I just tend to avoid curly braces when possible because I do a lot of work in F# and python. pValues are parameters, tValues are local scoped, Values are class properties. =) \$\endgroup\$ – Kenneth Posey Sep 5 '14 at 20:02
  • \$\begingroup\$ must be f# name scheme? braces aren't a good habit with Python though, right? \$\endgroup\$ – Malachi Sep 5 '14 at 20:09
  • 1
    \$\begingroup\$ It's a naming system that everybody I've met has strong opinions about one way or another. If I'm writing public-domain code then I tend to stick to more traditional naming conventions, but for my own projects I don't stress over it. It works for me and it's easily understandable if/when I ask for help as soon as the person knows what's going on. =) \$\endgroup\$ – Kenneth Posey Sep 5 '14 at 20:15
  • 2
    \$\begingroup\$ I think the important thing to ask is does it make mistakes look wrong? \$\endgroup\$ – RubberDuck Sep 5 '14 at 20:17
  • 2
    \$\begingroup\$ Especially concerning class properties and parameters, my method absolutely makes problems look wrong. For example, if I'm assigning to a value with no prefix and a capital letter, I know it is going to change something that's out of the current scope of the function. IE: a potential bug. If I assign to something with "p", then I know I just changed a parameter, and that could affect functionality later that are depending on the parameter being the data given to the function. If you have functions passing values directly to other functions then it's hugely useful to avoid that "gotcha". \$\endgroup\$ – Kenneth Posey Sep 5 '14 at 20:27

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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