16

We've got a type which has an implicit string operator. It looks like this:

public class Foo
{
    readonly string _value;

    Foo(string value)
    {
        _value = value;
    }

    public static implicit operator string(Foo foo)
    {
        return foo._value;
    }

    public static implicit operator Foo(string fooAsText)
    {
        return new Foo(fooAsText);
    }

}

We've just had a scenario where the instance passed to the implicit operator was null. Obviously, we ended up with a NullReferenceException.

I thought this was fair enough, after all, what IS the string representation of a type that is null - hard to say - so the exception seemed valid and something we shouldn't intercept/suppress/handle/ignore. My reasoning was along the lines of 'someone's given me a null, why should I return null. I don't do this in other methods'. I thought, 'I know, I'll just check for null before converting it', something like:

string s = foo == null ? null : foo;

But that didn't work, because it's now converting to a string before the comparison to null. Of course, this comparison would work:

Foo f = null;
string s;
if (f != null)
{
    s = f;
}

... but that's just ugly.

I read the section in Essential C# 5 4th edition (a 'Rough Cuts' book on Safari) and it says:

DO NOT throw exceptions from implicit conversions.

This says don't throw exceptions, but it leaves me wondering should I suppress exceptions?

The most obvious thing to do is to jump into the method and change it to:

public static implicit operator string(Foo foo)
{
    return foo == null ? null : foo._value;
}

Problem solved. But I feel dirty. I feel like I'm hiding a potential bug by checking for null. Am I being paranoid/anal/stupid?

Steve Dunn
  • 21,044
  • 11
  • 62
  • 87
  • 1
    it all depends on your use case. do you want the `string` to be null? – Daniel A. White Nov 29 '12 at 14:31
  • 1
    Good point. Ideally we'd like never to come across a null instance of a `Foo`. Hmmm... Perhaps it should be a struct... – Steve Dunn Nov 29 '12 at 14:34
  • 5
    @SteveDunn: One important thing: A `NullReferenceException` always indicates a bug in the code that raises the exception, *not* in calling the code. So simply not checking `foo` for `null` is a bug in the operator. – Daniel Hilgarth Nov 29 '12 at 14:42

4 Answers4

13

What I would recommend in this case is that the operator which can fail should be explicit rather than implicit. The idea of an implicit conversion is that it is widening (i.e. it will never fail.) Explicit conversions, on the other hand, are understood to sometimes be able to fail.

Dan Bryant
  • 27,329
  • 4
  • 56
  • 102
6

I would want Foo to act the same as string. So, I'd expect

Foo foo = null;
string.Equals(null, foo);

to be true.

string.Equals will try to cast foo to a string so it will call the implicit operator. The null-valued foo should expose the same value as a null-valued string would, which is just null.

So, if you do want to have an implicit operator, I'd implement it as:

public static implicit operator string(Foo foo)
{
    return foo?._value;
}
comecme
  • 6,086
  • 10
  • 39
  • 67
0

It is perfectly fine to "supress" exception within implicit conversion operators. In fact, that is the recommended approach, as you've pointed out.

Consider this scenario:

Foo actualFoo = null;
string stringFoo = actualFoo;

If this compiles just fine, why would you want the second line to trow an exception during runtime? That would make no sense.

The same goes for this:

string stringFoo = null;
Foo actualFoo = stringFoo;

If you are defining both implicit conversion operators like that, it means you want to treat and use type Foo the same way as you would use type string. In that case both of the scenarios above are perfectly valid constructs (your compiler confirms that) and should yield null rather than trowing an exception.

Now, as Dan have pointed out, if this is not what you want, then you'll need to define an explicit conversion instead.

Steve Dunn
  • 21,044
  • 11
  • 62
  • 87
Milos Mrdovic
  • 1,441
  • 1
  • 17
  • 30
-3

You are trying to cast null to the type Foo as the type of both side of : must be equal.

string s = foo == null ? null : foo;

So you should use

string s = foo == null ? (string)null : foo;

or

string s = foo == null ? null : (string)foo;
jrswgtr
  • 2,287
  • 8
  • 23
  • 49