Is checking for type instead of null a reasonable choice?

Question

One of my (senior) coworkers does something really strange in his code.

Instead of checking a variable for null, he checks for the type. And because

null is FooType

actually returns false, this works.

public class Foo
{
    private string _bar = null;

    public string Bar
    {
        get
        {
            // strange way to check for null
            return (_bar is string) ? _bar : "";
        }
        set { _bar = value; }
    }
}

I think this is bad coding and Resharper seems to agree with me. Is there any reason to write the check this way?

Is this a valid way to check the variable? Or can this be considered bad style or maybe even harmful in some special cases?

I don't want to confront him unless I am sure that this actually does not make sense.

Answer 1

This is not a good way. A better way would be to just:

return _bar ?? string.Empty;

Is it clear when you read your colleagues code that he is looking for nulls? No, then it isn't a good choice. Probably what the "is" operator will do first is just check for null and then return false. So it becomes much cleaner to just do that yourself. Or just use the null-coalescing operator

Answer 2

I think this code is completely confusing and would never use it. _bar is declared as a string so this type check is just begging people to not understand the code.

Answer 3

Yeah that's a little odd. Why not just write:

return _bar ?? "" ;

When I need to do something like this, I have a little class to handle these details:

public class DefaultableValue<T>
{
    private T m_Value = default(T);
    public T Value
    {
        get
        {
            if (IsInvalidPredicate(m_Value))
            {
                m_Value = IfDefaultValueFunc();
            }
            return m_Value;
        }
    }
    private Predicate<T> IsInvalidPredicate { get; set; }
    private Func<T> IfDefaultValueFunc { get; set; }
    public static implicit operator T(DefaultableValue<T> property)
    {
        return property.Value;
    }
    public DefaultableValue(Predicate<T> isInvalidPredicate,Func<T> ifDefaultFunc)
        : this(default(T), isInvalidPredicate, ifDefaultFunc)
    {
    }
    public DefaultableValue(T initValue, Predicate<T> isInvalidPredicate, Func<T> ifDefaultFunc)
    {
        this.m_Value = initValue;
        this.IsInvalidPredicate = isInvalidPredicate;
        this.IfDefaultValueFunc = ifDefaultFunc;
    }
}

Then my class looks like

class Test
{
    DefaultableValue<string> AString { get; set; }

    public Test(string initialValue)
    {
        this.AString = new DefaultableValue<string>(initialValue, 
            (value) => string.IsNullOrWhiteSpace(value),
            () => string.Empty);
    }
}

....
var test = new Test(null);
var someString = test.AString; // = "" not null