Base Class

[shapper]

Hello,

I am trying to create a common "SessionProvider" that a SessionService
use.

I came up with the following Session provider:

// SessionProvider
public class SessionProvider<Child> where Child :
SessionProvider<Child>, new() {

private static String Key {
get { return typeof(SessionProvider<Child>).FullName; }
} // Key

private static Child Value {
get { return (Child)HttpContext.Current.Session[Key]; }
set { HttpContext.Current.Session[Key] = value; }
}

public static Child Current {
get {
var instance = Value;
if (instance == null)
lock (typeof(Child)) {
instance = Value;
if (instance == null)
Value = instance = new Child();
}
return instance;
}
} // Current

} // SessionProvider

Then I have for each project the following:

public interface ISessionService {
DateTime Appointment { get; set; }
} // ISessionService

public class SessionService : SessionProvider<SessionService>,
ISessionService {
public DateTime Appointment { get; set; }
} // SessionService

What do you think?

Then I inject the Session Service having the following on my Structure
Map configuration:

For<ISessionService>().HttpContextScoped().Use(() => new
SessionService());

Maybe I should create an interface for SessionProvider instead of
SessionService?
How should I do this?

Now to get a session value or save it I use the following on my
controller.

sessionService.Current.Appointment = DateTime.UtcNow;
DateTime when = sessionService.Current.Appointment;

The naming SessionService and SessionProvider are just what I came up
with ...
Any suggestion about the naming?

Thank You,
Miguel

 

[Peter Duniho]

[...]
Maybe I should create an interface for SessionProvider instead of
SessionService?
How should I do this?

You can't create an interface for static members. This is perhaps one
argument in favor of using a factory pattern instead (as I suggest below).

[...]
Now to get a session value or save it I use the following on my
controller.

sessionService.Current.Appointment = DateTime.UtcNow;
DateTime when = sessionService.Current.Appointment;

What is "sessionService" here? Static members cannot be accessed
through instance references. Do you mean "SessionService" instead?

As for the general design:

• First issue is that IMHO self-referential generics should be
avoided except where there is a _strongly_ compelling reason to use
them. They can be very useful in just the right scenario, but they make
it harder to reason about the relationships of the types and can cause
maintenance issues.

This seems to me to be the most significant problem. It seems to me
that the functionality you're implementing could be better-implemented
using a memoized factory pattern. I.e. a factory method or class that
caches returned instances.

• Also, your "Current" property is flawed in a variety of ways:
— Most important, you should not lock on a Type instance; locking
should happen only using a private object instance.
— Also, you also might as well get rid of the double-check lock;
it's doubtful that even if you could implement it correctly, you really
need the minuscule improvement in performance it gives you, and because
you don't have control over where the data is stored, you can't ensure
that it's stored as a volatile, which is needed in order for the
double-check lock to be reliable.
— Finally, also because you don't have control over the storage,
your lock isn't reliable anyway, because there could be some other
change to the value elsewhere if some other code decides to use that
same key. Of course, ideally you will simply make sure no such code
exists. But it's a serious maintenance problem to rely on that being
true, especially since the key you've chosen is one easily reproduced by
other code.

At the very least, IMHO you should consider constructing a key that is
more than just the type name of the generic type parameter. For
example, concatenate both the type parameter's type name with the
generic type's type name.

• Also, you should use "T" as the type parameter for the generic
type. In cases where there are more than one type parameter,
descriptive names should be used, but even there the names should start
with "T". Doing it this way helps make clear within the implementation
itself when a type parameter is being used as a type.

Hope that helps.

Pete