language-ext icon indicating copy to clipboard operation
language-ext copied to clipboard

(Remove) Implicit conversion to `Error`

Open StefanBertels opened this issue 2 years ago • 3 comments

After updating to LanguageExt 4.x I ran into a problem caused by implicit convertion from string to Error because I have a method with two overloads and both overload types can be implicitly converted from string. (I'll create a workaround for now.)

While I appreciate some of LanguageExt implicit conversions (e.g. assign T to Option<T>) I think this one is "too much".

IMO the general rule should be that implicit conversions should not lead to suboptimal code in "other places". I don't think that string is somehow equivalent to Error (same for (int, string)).

IMO it should be no problem to implicit convert Exception to Error.

I think problem surface will increase if Error is used outside of LanguageExt itself.

StefanBertels avatar Nov 10 '21 17:11 StefanBertels

You may be right, after the millionth time writing Error.New(...) I probably just got a bit frustrated. Let me sit on this for a few days to consider it.

louthy avatar Nov 11 '21 11:11 louthy

To be honest the problem is 50% caused by an implicit conversion I built for same reason, i.e. this class:

public class LoggableString 
{
    public readonly string FormatText;
    public readonly object?[]? Arguments;

    public LoggableString(string formatText, object?[]? arguments = default)
    {
        FormatText = formatText;
        Arguments = arguments;
    }

    public static implicit operator LoggableString(FormattableString formattable) => new(formattable.Format, formattable.GetArguments());
    public static implicit operator LoggableString(string str) => new(str);

    public override string ToString() => Arguments == null ? FormatText : FormattableStringFactory.Create(FormatText, Arguments).ToString();
}

This class allows me to have logging functions (.Verbose, .Debug, .Warn ...) that take simple strings and interpolated strings and to use structured logging for interpolated ones -- all with one function. This just collides with an overloads that accepts Error (I have another overload for e.g. .Error that accepts Exception).

Anyway, these things come to my mind:

  1. Would be nice (not just here) to have an option to enable or disable implict conversions. AFAIK implicit conversion cannot be done using e.g. Extension methods (as with all operators) which would be a great solution.

  2. If it's not possible to "disable" implicit convertion the implicit conversion from string to Error will reduce the usefulness of Error in user code. If you want to support Error in user code there shouldn't be a (relevant) chance of new problems like this. While upgrading LanguageExt I noticed that Error.Code is no longer used in Error.ToString().

  3. It's impossible/utopic, but if we just could add some ! after an expression to turn explicit conversions into implicit ones (target typed), this could be a great c# language feature. We could pass "this is a problem"! to a method .Debug(Error err) then. I mean: a! would be like some (var) a... I guess this even cannot be done by some (generic) extension function because we would need a specific target type. Best next (possible) thing is (Error) "test", I guess.

StefanBertels avatar Nov 11 '21 16:11 StefanBertels

To be honest, I really like that we don't have to write Error.New everywhere right now. To me it was a blessing change :man_shrugging:

kirill-gerasimenko-da avatar Nov 13 '21 07:11 kirill-gerasimenko-da