Two-way implicit conversion operators are a bad idea

[DavidLievrouw]

We are using implicit conversion operators in several models, e.g. in KeyId

        /// <summary>
        ///     An implicit conversion operator for this type from string.
        /// </summary>
        public static implicit operator KeyId(string value) {
            if (!TryParse(value, out var keyId)) {
                throw new FormatException($"The specified value ({value ?? "[null]"}) is not a valid string representation of a {nameof(KeyId)}.");
            }

            return keyId;
        }

        /// <summary>
        ///     An implicit conversion operator for this type to string.
        /// </summary>
        public static implicit operator string(KeyId id) {
            return id.ToString();
        }

This is generally considered a bad idea: You effectively lose type-safety when passing models as method arguments. For example, the following call will not cause a compile-time error:

public void DoSomething(KeyId id, ShortGuid guid) {}

service.DoSomething("", "");

In this example, both string values are implicitly converted to KeyId and ShortGuid. But because of the two way conversion, the following works too:

public void DoSomething(KeyId id, ShortGuid guid) {}

KeyId id = new KeyId("one");
ShortGuid guid = ShortGuid.NewGuid();
service.DoSomething(guid, id); // Wrong order!

This compiles, because both arguments are first implicitly converted to string and then back to the type of the argument.

And that, my friends, is very confusing. This should not be possible.

There is a relatively easy fix, however: Just make one of the conversion operators explicit. But that is a breaking change for consumers.

[DavidLievrouw]

Fixed in commits 302f634fe2a64a1a31111a3060dec43d6a29a72f and b79cff23a7501e2dcbf89b358288d53ccc5ceb5a and 761309154635f8742f506c960b03a17263404da8

[DavidLievrouw]

Will be included in release v9.0.0.