DavidArno / SuccincT

Discriminated unions, pattern matching and partial applications for C#
MIT License
267 stars 15 forks source link

Remove Maybe type and change Option to be a readonly struct #57

Closed DavidArno closed 4 years ago

DavidArno commented 6 years ago

This is a major breaking change:

  1. Any code using Maybe<T> will need to change to use Option<T>.
  2. In line with the conversation in #51, the way Option<T> handles null changes: i. Option<string>.Some(null) will throw an ArgumentNullException. ii. As the type supports implicit casting of T to Option<T>, casting null in this way will result in a None:
    Option<string> x = null`
    AreEqual(Option<int>.None(), result); // test passes
DavidArno commented 4 years ago

Have abandoned this idea as read only structs are just weird.

SugoiDev commented 4 years ago

Do you have any insight on the weirdness you found? I was planning some changes on a codebase using readonly structs soon.

DavidArno commented 4 years ago

Well yes, I do. And having gone full circle, I'm re-opening this.

The reason I claimed that readonly structs are weird is due to the distinction between "read only" and "immutable". In C#, readonly denotes that the value of that item cannot change. This means that I might have the class,

class C
{
    readonly List<int> _x = new List<int>();

    void ReadonlyNotImmutable() => _x.Add(1);
}

The "value" of _x can't change, ie it is fixed to referring to the same List<int> object. But the contents can change, ie it's readonly not immutable.

Readonly structs apply this same rule, but it creates odd results. So for example, the following code:

class FakeTuple
{
    public int a;
    public int b;
}

readonly struct S
{
    readonly (int a, int b) _x;
    readonly FakeTuple _y;

    public S(int n)
    {
        _x = (n, n);
        _y = new FakeTuple { a = n, b = n };
    }

    int X { set { _x.a = value; } }           
    int Y { set { _y.a = value; }}
}

gives an error on _x.a = value; but not on _y.a = value;. Further, despite marking it as readonly, I can still have setters. So I really struggled with the idea that it's readonly as to me, that ought to rule out setters.

But what I was missing is the simple idea that, just like with a readonly field, a readonly struct guarantees its value cannot change. And the value of a struct is the value of its fields. So the tuple, being a struct itself, cannot change the value of its fields as that would change its own value and thus the value of the struct, S. Likewise the reference for the FakeTuple field can't change, but items it contains can, as they aren't part of the struct's value. And setters are allowed, as long as they don't change the value of the struct.

So having had that lightbulb moment, I've gone back and re-introduced that readonly struct for Option. :smile:

DavidArno commented 4 years ago

Adding a checklist here for all the types that can become readonly structs:

DavidArno commented 4 years ago

Maybe<T> is gone and Option<T> is now a read-only struct.

Closing as implemented.