11

I've borrowed the code below from another question (slightly modified), to use in my code:

internal class PositiveDouble 
{
      private double _value;
      public PositiveDouble(double val) 
      {
          if (val < 0)
              throw new ArgumentOutOfRangeException("Value needs to be positive");
          _value = val;
      }

      // This conversion is safe, we can make it implicit
      public static implicit operator double(PositiveDouble d)
      {
          return d._value;
      }
      // This conversion is not always safe, so we're supposed to make it explicit
      public static explicit operator PositiveDouble(double d)
      {
          return new PositiveDouble(d); // this constructor might throw exception
      }
}

The original author of this code correctly adheres to the warnings given in MSDN's implicit & explicit documentation, but here's my question: Is explicit always necessary in potentially exceptional code?

So, I've got some types in my code (e.g. "Volume") that derive from PositiveDouble and I'd like to be able to set instances conveniently like the first line below:

Volume v = 10; //only allowed by implicit conversion
Volume v = new Volume(10)  //required by explicit conversion, but gets messy quick

Being forced to use explicit casts everywhere makes the code much less readable. How does it protects the user? In the semantics of my program, I never expect a Volume to be negative; indeed, if it ever happens I expect an exception to be thrown. So if I use an implicit conversion and it throws, what "unexpected results" might clobber me?

Community
  • 1
  • 1
kmote
  • 16,095
  • 11
  • 68
  • 91
  • Seems to me changing your conversion operator to implicit should be fine here. – Botonomous Sep 25 '14 at 19:43
  • 1
    It is recommended, in particular in a public API. It prevents people from using the cast without noticing it may throw, saving time in debugging. Making people notice the cast will avoid trouble. They say "oh, so you think you are clever making all cast implicit?" and you say "If you know what you are doing, you won't have problems" - imagine an stove that starts when something is on it, because "Being forced to start the stove every time makes cooking much less efficient", and the manufactures respond to claims with "then don't put things on it" - just less dramatic and without the burns. – Theraot Sep 25 '14 at 22:47

2 Answers2

15

The C# language specification says under 10.10.3 Conversion operators:

If a user-defined conversion can give rise to exceptions (for example, because the source argument is out of range) or loss of information (such as discarding high-order bits), then that conversion should be defined as an explicit conversion.

Also, from MSDN: implicit (C# Reference):

In general, implicit conversion operators should never throw exceptions and never lose information so that they can be used safely without the programmer's awareness. If a conversion operator cannot meet those criteria, it should be marked explicit.

Considering this, your operator PositiveDouble(double d) should not be marked implicit, as Volume v = -1 will throw an exception.

So to answer your question:

Is explicit always necessary in potentially exceptional code?

No, it's not necessary, but it should.

Bordering on opinion-based: if your code is the only code using this conversion, and you find implicit conversion easier to read and/or maintain, feel free to use that.

As for

How does it protect the user?

See MSDN: explicit (C# Reference) mentions:

If a conversion operation can cause exceptions or lose information, you should mark it explicit. This prevents the compiler from silently invoking the conversion operation with possibly unforeseen consequences.

I can't really fathom when this would occur, but again, if you think you never convert from a negative double in your code anywhere, this should not ever throw.

Jack
  • 2,223
  • 13
  • 23
CodeCaster
  • 147,647
  • 23
  • 218
  • 272
  • 2
    *"I can't really fathom when this would occur"* -- It can easily happen when passing objects with custom conversions to framework methods that accept such arguments. It's not really obvious in the code that this will happen unless you (a) know exactly what argument type(s) the framework method accepts, and (b) know that the implicit conversion exists. – cdhowie Sep 26 '14 at 03:13
13

Conversions should be implicit only if they will always succeed. If they can fail or cause loss of information, they should be explicit, because this requires the caller of the conversion operation to intentionally indicate that they want this conversion and therefore are prepared to deal with the outcome, whatever that may be.

We can see this in the framework primitive numeric types; assigning an int to a long is an implicit conversion, because it will always succeed with the intended result. Going the other way can cause an OverflowException in checked context, and can cause truncation (loss of information) in unchecked context, so you are required to indicate that you intended this conversion by explicitly casting.

cdhowie
  • 158,093
  • 24
  • 286
  • 300