HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Session state wrapper, extending an existing application

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
applicationwrapperstatesessionexistingextending

Problem

I'm adding additional functionality to an existing MVC .net application, and to help prevent or at least reduce repeated reads to the dB I'm dumping a few custom entities in session. I'm limiting what can be stored in session to say a max of 5 objects for now.

Given below is the code and it seems generic enough and works fine locally on my box; any suggestions to improve this and session is In-Proc but if it does get stored in a persistent medium say SQL server then I would need to customize this further - decorate custom object properties w/XML element attributes

```
public class QuestionModelSessionStateProvider : IQuestionModelSessionStateProvider
{
const int MaxSize = 5;

public T GetValue(string key)
{
HttpSessionState session = GetSessionState();

T returnValue = default(T);

if (session == null) return returnValue;

if (session["QuestionModelStore"] != null)
{
var questionModelStore = (List>)session["QuestionModelStore"];
foreach (var t in questionModelStore.Where(t => t.Key.Equals(key)))
{
returnValue = t.Value;
}
}

return returnValue;
}

public void SetValue(string key, T value)
{
HttpSessionState session = GetSessionState();

if (session == null) return;

List> questionModelStore;

if (session["QuestionModelStore"] == null)
{
questionModelStore = new List> { new KeyValuePair(key, value) };
session["QuestionModelStore"] = questionModelStore;
}
else
{
questionModelStore = (List>)session["QuestionModelStore"];
questionModelStore.Add(new KeyValuePair(key, value));
}

if (questionModelStore.Count > MaxSize)
{
questionModelStore.RemoveAt(0);
}
}

public void RemoveValue(string key)
{
HttpSessionState session = GetSessionState();
int ind

Solution

There are 4 places where you have code like,

if (session != null)
{
    List> questionModelStore;

    if (session["QuestionModelStore"] == null)
    {
        questionModelStore = (List>)session["QuestionModelStore"];


It would be neater if these were a common subroutine:

List> GetQuestionModelStore()
{
    var session = HttpContext.Current.Session;
    if (!session)
        return null;
    return (List>)session["QuestionModelStore"];
}


That would simplify code such as:

public T GetValue(string key)
{
    var cached = GetQuestionModelStore();
    if (cached == null)
        return default(T);
    return cached.FirstOrDefault(t => t.Key.Equals(key));
}


It might be slightly faster to use a Queue than a List, because a Queue is designed to let you enqueue at one end and dequeue at the other.

You're not quite implementing a LRU algorithm: if you want to do that, when you find an item in the cache you should move the item to the most-recently-used end of the list.

There's one bug I see, which is that your methods are generic: the first time you call SetValue it creates a List>; unless all your SetValue and GetValue methods use the same type of T this will cause a run-time error (casting between different types of T). If all your SetValue and GetValue methods do use the same type of T then you might as well specify what T is instead of making it generic.

Code Snippets

if (session != null)
{
    List<KeyValuePair<string, T>> questionModelStore;

    if (session["QuestionModelStore"] == null)
    {
        questionModelStore = (List<KeyValuePair<string, T>>)session["QuestionModelStore"];
List<KeyValuePair<string, T>> GetQuestionModelStore<T>()
{
    var session = HttpContext.Current.Session;
    if (!session)
        return null;
    return (List<KeyValuePair<string, T>>)session["QuestionModelStore"];
}
public T GetValue<T>(string key)
{
    var cached = GetQuestionModelStore();
    if (cached == null)
        return default(T);
    return cached.FirstOrDefault(t => t.Key.Equals(key));
}

Context

StackExchange Code Review Q#43546, answer score: 2

Revisions (0)

No revisions yet.