tags:

views:

78

answers:

2

I have a class called EditMapUtilities.

Here are some class properties that I want to persist:

public class EditMapUtlities
{
    public static Boolean isInitialEditMapPageLoad
    {
        get { return SessionHandler.isInitialEditMapPageLoad; }
        set { SessionHandler.isInitialEditMapPageLoad = value; }
    }

// REST OF CLASS NOT GERMAIN TO DISCUSSION AND OMITTED
}

Here is my SessionHandler Class following the pattern from this post Static Session Class and Multiple Users:

using System.Web.SessionState;


public static class SessionHandler
{
    private static HttpSessionState currentSession
    {
        get
        {
            if (HttpContext.Current.Session == null)
                throw new Exception("Session is not available in the current context.");
            else
                return HttpContext.Current.Session;
        }
    }

    //A boolean type session variable
    private static string _isInitialEditMapPageLoad = "EditMapInitialPageLoad";
    public static bool isInitialEditMapPageLoad
    {
        get 
        {
            if (currentSession[_isInitialEditMapPageLoad] == null)
                return true;
            else
                return (Boolean)currentSession[_isInitialEditMapPageLoad];    
        }
        set
        {
            currentSession[_isInitialEditMapPageLoad] = value;
        }
    }
}

I am still learning OOAD. I want to keep relevant properties with relevant classes. I also want to keep all Session stored variables in one place for ease of maintenance and to encapsulate the session keys and calls.

I feel like my design is too coupled though. How can I make it more loosely coupled? Is my editMapUtilities class too tightly coupled to the SessionHandler class? How would you do it better?

A: 

There is nothing wrong with this. Don't worry about coupling so much - classes have to work together just like bricks. People don't worry when their bricks are tightly coupled. Session and ASP.Net go hand in hand like bricks and mortar. In other words, to write a web application, one must add statefulness to HTTP.

I would say that in this case you might not need to use the session. I tend to use the ViewState for values that I don't care if the client can change and reserve the Session for more sensitive data (entity ids, role based variables)

matt-dot-net
@matt -- I understand what you are saying in spirit, but in my professional experience (having actually worked for a masonry outfit) tight coupling will almost always lead to maintenance headaches later on all but the most trivial of projects. Loose coupling is very easy to achieve in most cases with a little thought and if you intent to Unit Test your application apart for having the entire Web Context spun up, then you absolutely ''Must'' code to abstractions. Also, bricks are not tightly coupled. Mortar is pretty easy to remove, and masons re-use bricks from torn down walls ;)
Josh
Okay, forget the masonry analogy. You need session to have a web app. If you are trying to talk yourself into rolling your own session, then you're doing it wrong.
matt-dot-net
It's good to be practicing design patterns, but this isn't a case to implement a SessionHandler
matt-dot-net
Matt -- I am mainly trying to prevent collisions from two classes setting variables in session and accidentally using the same key names. So I strongly type the session key names by using the session handler class.I like the KISS (keep it simple stupid) approach to things and try to have that with good OOAD principles.I may look at using ViewState, but I am working on the server and holding results of a class run on the server. There is no need to push the results down to the client. ViewState is stored on a hidden field on a Page object.session is stored in memory on the server
shawn deutch
+2  A: 

Your intuition is pretty good in that your classes are indeed very tightly coupled to one another. I'm going to be referencing Bob Martin's Principles of Object Oriented Design.

Following the Single Responsibility Principle, you want to make sure that you don't have to update both of these classes if one of them changes. Right now both classes know a little too much about each other.

We can start with EditMapUtilites and refactor access to the session handler to more accurately convey the intent. Mainly, application state scoped to the user. Also, we can change how we access those values to make it less coupled by using methods that accomplish the same goal.

public class EditMapUtlities
{
    //Omitting the wireup details for now
    private SessionHandler ApplicationState {get; set;}

    public static Boolean isInitialEditMapPageLoad
    {
        get { return ApplicationState.GetValue<Boolean>("EditMapInitialPageLoad"); }
        set { ApplicationState.SetValue("EditMapInitialPageLoad", value); }
    }

// REST OF CLASS NOT GERMAIN TO DISCUSSION AND OMITTED
}

And the SessionHandler class would now contain two new methods:

public static T GetValue<T>(String key)
{
    return currentSession[key] ?? default(T);
}

public static void SetValue(String key, Object val)
{
    currentSession[key] = val;
}

Ok, so one class is now slightly more verbose, but your session class is simpler and more flexible. Adding additional values is simple, but you may ask yourself why not just use straight session?

The reason we restructured the classes this way was to get us one step closer to a clean break between them. By accessing our SessionHandler class through a property we can now take our refactoring one step further and employ the Dependency Inversion Principle.

Instead of relying on a concrete implementation of SessionHandler, why don't we abstract the storing of Application State variables behind an interface. One that looks like this:

public interface IApplicationState
{
    public T GetValue<T>(String key);
    public SetValue(String key, Object val);
}

Now we can simply change over our property inside of EditMapUtlities to use the interface, and pass this in through the constructor using good ol' manual Dependency Injection.

public class EditMapUtlities
{
    //Omitting the wireup details for now
    private IApplicationState ApplicationState {get; set;}

    public EditMapUtilities(IApplicationState appState)
    {
        if(appState == null) throw new ArgumentNullException("appState");

        ApplicationState = appState;
    }

    public static Boolean isInitialEditMapPageLoad
    {
        get { return ApplicationState.GetValue<Boolean>("EditMapInitialPageLoad"); }
        set { ApplicationState.SetValue("EditMapInitialPageLoad", value); }
    }

// REST OF CLASS NOT GERMAIN TO DISCUSSION AND OMITTED
}

You can easily refactor your SessionHandler class to implement the interface and not be static:

public SessionHandler: IApplicationState { //Implementation }

Or if you need to maintain backward compatibility with other areas of code, you could create a wrapper class, and leave SessionHandler static:

public SessionHandlerWrapper: IApplicationState
{
    //Call SessionHandler from within this class
}

Now it's true that you will have to manually create your concrete instance of IApplicationState and pass it into EditMapUtilities, but now neither class is dependent on the other. In fact, if you ever wanted to store this in the database in the future, you would simply have to write a new class that implemented IApplicationState. Only the calling code would have to change to use the new database class instead of the session based one.

Now you can enjoy your new loosley coupled architecture and the minimized changes you will have to make to add new functionality.

Josh
I needed to persist variable scope for the current session. So I use global variables (eg. session variables) The whole reason I came up with the session handler class was to be able to strongly type my session key(variable names) to prevent collisions. You could add an enum to the session handler class and pass that as the key instead of the string value. The string Key value is to broad and open to collision.Interesting use of generics so I don't have to worry about types in the session handler class.
shawn deutch
@Shawn -- My original solution does in fact include the use of an Enum that maps to keys. I didn't include it here because I thought it would be too much noise surrounding the solution of breaking the coupling between classes.
Josh