Dedicated thread for arguing about the autocast from O to ContinuousMap

[AdamNorberg]

ContinousMap.cs (engine/calculus) currently specifies an automatic conversion from arbitrary types to a constant ContinuousMap taking a context-deduced input type (discarded) to the type of the converted object. This will make the code much more difficult to maintain.

Accepting literally every type as a ContinuousMap of some sort makes it easy to use the wrong variable in an expression, swap variables around, or get unexpected types out of an expression because of an unexpected ContinuousMap conversion. Silent reinterpretation of data makes code innately hard to read, because nothing makes it immediately obvious that a conversion is happening, other than being already familiar enough with the code that they know the types are incompatible.

Automatic type conversions are usually a mistake. (Admittedly, I'm a Go programmer at heart, so of course I think this.) I want the compiler to tell me when I am trying to use the wrong data for something, and the type system is the primary way it can do so. An automatic type conversion that, in a suitable context, could be applied to every type is absolutely begging for trouble and expressions that weren't intended to have anything to do with ContinuousMap could be deduced by the compiler to refer to it.

To deal with the verbosity of constructing ConstantFunction<float, float> everywhere, I propose two alternatives:

1. public static class Abbreviations, intended for use via using static engine.Abbreviations, with static convenience functions with really short names for stuff like this: for example, public static ContinuousMap<object, T> <T> C(T t) { return ConstantFunction<object, T>(t);} - that's shorter than I'd like, I'd prefer ConstFxn, but we can do whatever works.
2. In issue #9 , we're considering introducing another type, flex, that will be either a double or a float depending on whether we want production code or to reduce precision to surface numerical stability threats. flex could have a read-only property, CFxn, that creates a constant function from flex.

Note that in both cases, I'm marking this as a ContinuousMap from object to the parameterized type. Currently, ContinousMap is not implemented to be covariant. This is easily fixed, and switching to Func or delegate will also make it appropriately covariant. This would allow a ContinuousMap<object, T> to be used in a field declared as ContinuousMap<S, T>, since C# would then know that this first type is "input-flavored" and a strictly broader type can safely be used whereever a narrower type is expected (and object is maximally broad). See https://docs.microsoft.com/en-us/dotnet/standard/generics/covariance-and-contravariance for more details.

[Lathreas]

I've put in some thought and analyzed the options. First of all, I am in full agreement that an implicit cast is ugly and dangerous and should be removed or constrained. You raise some good alternatives, although they are not yet fully satisfying to me since they too introduce some additional clutter that any user of the engine (including frontend work) will have to deal with.

Looking at my own code a bit, I think another viable solution might be a rather simple one. Since the conversions only really happen at engine interface functions (such as class constructors) that are exposed to users of the engine, we could add overloading constructors for each type that takes a ContinuousMap<I, O> such that they can also take e.g. a float (or a Real and/or double type in any new code we write).

Currently this is implemented as follows in the soon-to-be-published branch, e.g. from Cylinder.cs:

        public Cylinder(Curve centerCurve, float radius)
        {
            this.CenterCurve = centerCurve;
            this.Radius = new ConstantFunction<Vector2, float>(radius);
            this.StartAngle = 0.0f;
            this.EndAngle = 2.0f * (float)Math.PI;
        }

        public Cylinder(Curve centerCurve, ContinuousMap<Vector2, float> radius)
        {
            this.CenterCurve = centerCurve;
            this.Radius = radius;
            this.StartAngle = 0.0f;
            this.EndAngle = 2.0f * (float)Math.PI;
        }

This works well in practice. The only reason I added an implicit cast was to reduce the code duplication we have here, since this happens in basically every file where we have a ContinuousMap as a parameter. We can reduce the code a bit by calling the second constructor from the first one, and possibly this is enough to reduce clutter enough while keeping the code safe and future-proof.

Do you think this is a reasonable idea, or will the code duplication become too much in the end?

I've also taken a closer look at the concepts of covariance and contravariance, and it might indeed be good to implement that as well considering we know for sure which types are "input-flavored" and which ones are "output-flavored".

[AdamNorberg]

100% on board with the overloaded constructors / functions approach, I think that's the most appropriate way to do it.

C# has a syntax for "translating" constructors to each other:

public Cylinder(Curve centerCurve, float radius)
    : this(centerCurve, new ConstantFunction<Vector2, float>(radius))
{
}

https://stackoverflow.com/questions/4009013/call-one-constructor-from-another has more patterns for avoiding code duplication in constructors.

[Lathreas]

Ah, fantastic! Then that's what I'll use. This looks like the most elegant and readable option.

[Lathreas]

(revisitable)