Is this redundant code?

I'm upgrading a system and am going through another developers code (ASP.NET in C#).

I came across this:

private ReferralSearchFilterResults ReferralsMatched
{
    get
    {
        if (Session[SESSION_REFERRAL_SEARCHFILTERRESULTS] == null || Session[SESSION_REFERRAL_SEARCHFILTERRESULTS].GetType() != typeof(ReferralSearchFilterResults))
            return null;
        else
            return (ReferralSearchFilterResults)Session[SESSION_REFERRAL_SEARCHFILTERRESULTS];
    }
    set
    {
        if (value == null)
        {
            Session[SESSION_REFERRAL_SEARCHFILTERRESULTS] = value;
        }
        else if (value.GetType() == typeof(ReferralSearchFilterResults))
        {
            Session[SESSION_REFERRAL_SEARCHFILTERRESULTS] = value;
        }
    }

}

Is checking the type on the setter unnecessary? Surely, if I set the property to something other than a ReferralSearchFilterResults object, the code wouldn't even compile? Am I missing something or am I right to think this can be achieved just by using:

set
{
    Session[SESSION_REFERRAL_SEARCHFILTERRESULTS] = value;
}

The original code prevents any subclasses of ReferralSearchFilterResults from being set or get to or from the property. This is because value.GetType() will return the actual Type of the object referenced by value. If that Type is a subclass of ReferralSearchFilterResults, then it will not equals typeof(ReferralSearchFilterResults).

I'm not sure of your context here, so I can't tell you whether that's correct behaviour or not. If it's intended behaviour, it does smell a bit dirty as it will silently ignore any assignments of subclasses. But I can't really judge without more context.

I think you're right - the setter shouldn't compile if provided with something of that cannot be implicitly cast to a ReferralSearchFilterResults.

For the get part, you can use

return Session[SESSION_REFERRAL_SEARCHFILTERRESULTS] as ReferralSearchFilterResults;

This returns the value if it can be casted to ReferralSearchFilterResults, otherwise null.

Jamie you are correct. The Type check on the Setter is unnecessary in this case because value must be a ReferralSearchFilterResults.

One other change you might consider is using the is and as keywords in place of comparing Type objects.

private ReferralSearchFilterResults ReferralsMatched
{
    get
    {
        if (Session[SESSION_REFERRAL_SEARCHFILTERRESULTS] == null || !(Session[SESSION_REFERRAL_SEARCHFILTERRESULTS] is ReferralSearchFilterResults))
            return null;
        else
            return Session[SESSION_REFERRAL_SEARCHFILTERRESULTS] as ReferralSearchFilterResults;
    }
    set
    {
        Session[SESSION_REFERRAL_SEARCHFILTERRESULTS] = value;
    }

}

Session variables are of type object, so you can store anything inside those. But in this case the setter itself prevents the programmer from assigned an other object type than ReferralSearchFilterResults and derived objects. So the check, as you pointed out, itself is unneccessary. Additionally it does not let a programmer assign a object that derives from ReferralSearchFilterResults.

But I would use Session.Remove rather than just setting the variable to null, because the session variable would still exists in the http context if only set to null.

So:

set
{
      if (value == null)
            Session.Remove(SESSION_REFERRAL_SEARCHFILTERRESULTS);
      else
            Session[SESSION_REFERRAL_SEARCHFILTERRESULTS] = value;
}

I can understand the type check in the get bit, but as you say, in the setter, you can't pass in anything that's not a ReferralSearchFilterResults, as the code would fail at the time of compilation.

(Could be some old habit, the other developer had)