CODEWITHSUNDEEP
c#asp.netsession
Christian Wattengård
Christian Wattengård

Reputation: 5681

Is this use of generics correct?

After getting let down by a simple codeline like

char _letter = (char)Session["currentLetter"];

failing when the session is reset, I found out I wanted to try something. (currentLetter keeps track of position in a user table)

public static class SessionManager
{
    public static T ValueOrDefault<T>(HttpSessionStateBase session, string sessionkey, T defaultvalue)
    {
        object item = session[sessionkey];
        if (item != null && item.GetType() == typeof(T))
        {
            return (T)item;
        }
        else
        {
            return (T)defaultvalue;
        }

    }
}

Is this very wrong? Or am I doing something right here? (The code works, btw)

Upvotes: 0

Views: 95

Answers (3)

Ran
Ran

Reputation: 6159

Some more comments:

  1. Instead of item.GetType() == typeof(T), you may have wanted to use:

    typeof(T).IsAssignableFrom(item.GetType()). This would let you support reading a type derived from T.

  2. That said, in your specific case, if I understand the scenario correctly, I would have returned the default value only if the condition item == null is true.

    In your current code, the caller may get the default value returned if he was asking for the wrong type, which might be confusing for the caller as he has no way of knowing if a value was available or not.

    I would risk an assumption that if a caller is trying to get a value using the wrong type, this is always due to a bug (and not for example due to invalid user input), and therefore I would prefer to have an InvalidCastException thrown on me, so that the bug can be fixed (instead of the bug being hidden by returning a default value).

Upvotes: 1

Random Dev
Random Dev

Reputation: 52280

this seems right to me - but you can remove the cast to T from

return (T)defaultvalue;

into

return defaultvalue;

as you defined defaultvalue as T.

Upvotes: 1

CodesInChaos
CodesInChaos

Reputation: 108820

I'd replace item.GetType() == typeof(T) with item is T. This way it supports giving you a value types as the base class of what you want. You might want to consider making your method an extension method too.

currentLetter keeps track of position in a user table

I think this should not be a session state in the first place. But without knowing more about what you mean by that it's hard be sure.

Upvotes: 1

Related Questions