A different kind of Wither: With(Optional...)

[bboyle1234]

Can we please have the ability to generate a single wither for all properties using optional parameters?

// The record class 
[Record]
public sealed partial class SomeObject { 
  public int Property1 { get; }
  public int Property2 { get; }
}

// Example code gen
partial class SomeObject { 
  public SomeObject With(Optional<int> property1 = default, Optional<int> property2 = default) { 
    return new SomeObject(property1.GetValueOrDefault(Property1), property2.GetValueOrDefault(Property2);
  }
}

// Example usage
class Usage { 
  public void Bar() { 
    var foo = new SomeObject(0, 0);
    var foo2 = foo.With(property2: 10);
  }
}

The class that allows the optional parameters

public struct Optional<T> {
	private readonly T value;
	private readonly bool isDefined;

	/// <summary>
	/// Initializes a new instance of the <see cref="Optional{T}"/> struct.
	/// </summary>
	/// <param name="value">The value to specify.</param>
	[DebuggerStepThrough]
	public Optional(T value) {
		this.isDefined = true;
		this.value = value;
	}

	/// <summary>
	/// Gets an instance that indicates the value was not specified.
	/// </summary>
	public static Optional<T> Missing {
		[DebuggerStepThrough]
		get { return new Optional<T>(); }
	}

	/// <summary>
	/// Gets a value indicating whether the value was specified.
	/// </summary>
	public bool IsDefined {
		[DebuggerStepThrough]
		get { return this.isDefined; }
	}

	/// <summary>
	/// Gets the specified value, or the default value for the type if <see cref="IsDefined"/> is <c>false</c>.
	/// </summary>
	public T Value {
		[DebuggerStepThrough]
		get { return this.value; }
	}

	/// <summary>
	/// Implicitly wraps the specified value as an Optional.
	/// </summary>
	[DebuggerStepThrough]
	public static implicit operator Optional<T>(T value) {
		return new Optional<T>(value);
	}

	/// <summary>
	/// Gets the value that was given, or the specified fallback value if <see cref="IsDefined"/> is <c>false</c>.
	/// </summary>
	/// <param name="defaultValue">The default value to use if a value was not specified.</param>
	/// <returns>The value.</returns>
	[DebuggerStepThrough]
	public T GetValueOrDefault(T defaultValue) {
		return this.IsDefined ? this.value : defaultValue;
	}
}

[amis92]

That's something I did consider, and also is associated with #46 (I think originally I opened this issue with exactly what you want in mind as well, but didn't expand in the description).

The main problem is, which Optional<T> to use. Generate an internal one? Require some package reference? Make it configurable how and which type to use?

I'd lean to requiring a package reference to be used for this feature, e.g. https://www.nuget.org/packages/Optional (the most popular one, I think) - but it's still not sitting well with me.

[bboyle1234]

So glad you're active on this. I made a PR, hope you like it.

[bboyle1234]

I made the PR before I read your comments here, by the way :)

[bboyle1234]

I read through the https://www.nuget.org/packages/Optional package code.
My thoughts about it: Well that's a very thoroughly-written library :)
But I don't think any part of except the single Optional object is relevant to this project.
I don't think we have use for any part of that project except the Optional<T> object itself.
Therefore I feel it's smarter to avoid the dependency and bloat and just include our own Optional class.
Yes, I agree it's not cool to include it in the RecordGenerator.Attributes project, but I didn't know where else to put it. Perhaps we need to change the name of that project 🤣 just kidding. Where would you like to have it dropped?

Features: I started by creating a new feature for it, but I couldn't figure out a good name for the feature and just included it with the existing Wither feature. Any ideas?

[amis92]

Some ideas:

• OptionalWither
• WithOptional
• WitherExperimental

---

I'm strictly opposed to generating our own Optional type. It has a lot of issues:

• Two projects with records will create two separate, incompatible types
• It's incompatible with any other Optional implementation
• It's arbitrary

If you're not fine with requiring dependency on another package, then I suggest making it configurable (maybe with some presets). My idea:

[assembly: RecordOptionalConfiguration(typeof(Optional<>), HasValueProperty="HasValue", ValueProperty="Value")]
// generates
// using OptionalNamespace from resolved Optional type
// ...
/// public Record With(Optional<string> name = default) => new Record(name.HasValue ? name.Value : Name);

// another supported style

[assembly: RecordOptionalConfiguration("Optional.Optional`1", HasValueMethod="HasValue"
// generates
// using Optional; from resolved Optional type
// ...
/// public Record With(Optional<string> name = default) => new Record(name.HasValue() ? name.Value() : Name);

That way we can "default" to the Optional's package values, but allow customization and maybe, additionally, provide a CodeFix that generates a sample simple implementation.

How do you feel about it?

[atifaziz]

I'd lean to requiring a package reference to be used for this feature, e.g. https://www.nuget.org/packages/Optional (the most popular one, I think) - but it's still not sitting well with me.

I'm strictly opposed to generating our own Optional type. It has a lot of issues:

• Two projects with records will create two separate, incompatible types
• It's incompatible with any other Optional implementation
• It's arbitrary

Have a look at my project called Optuple. It's precisely designed to compensate for the lack of an option type in the BCL, but it does so by giving you option-like semantics over (bool, T) without taking any dependency. In other words, you just pretend that (bool, T) is option. From the project's README:

type Option<T> = | None | Some of T

Optuple, however, does not define any such type. Instead it primarily supplies extension methods for any (bool, T) (like Match, Map and more) to be treated like an option type. Suppose a value x of type T, then (true, x) will be treated like Some x and (false, _) will be treated like None. Note that in the none case, when the first element is false, Optuple completely ignores and discards the second element.

The benefit is:

A library that wants to expose optional values needs no dependency on Optuple. It can just expose (bool, T) for some type T. The client of the library can use Optuple independently to get “optional semantics”.

Once you make each With parameter a (bool, T), where you substitute T for the actual property type, you can then skin the implementation many ways. Here's an example using the Or extension:

public SomeObject With((bool, int) property1 = default, (bool, int) property2 = default) =>
    new SomeObject(property1.Or(Property1),
                   property2.Or(Property2));

Note how default acts as none for each parameter. Using just plain C#, you could go with pattern-matching instead of relying on extension methods like Or:

public SomeObject With((bool, int) property1 = default, (bool, int) property2 = default) =>
    new SomeObject(property1 is (true, var someProperty1) ? someProperty1 : Property1,
                   property2 is (true, var someProperty2) ? someProperty2 : Property2);

And if recursive pattern-matching is not an option (pun, pun😅), then you could go with naming the tuple/option elements and just use the good old ternary conditional operator (?:):

public SomeObject With((bool HasValue, int Value) property1 = default,
                       (bool HasValue, int Value) property2 = default) =>
    new SomeObject(property1.HasValue ? property1.Value : Property1,
                   property2.HasValue ? property2.Value : Property2);

My point in the end is (bool, T) does the job just as well and could be a reasonable compromise. You take no dependency on an external library (and even using Optuple is a consumer's choice entirely), introduce no new type and I don't think anyone would be too confused by the signature. Granted, it could seem strange at the call site to see obj.With((true, "foo"), (true, "bar")) but that can be taken care of by adding a one-liner:

static (bool, T) Some<T>(T value) => (true, value);

that's just sugar giving you obj.With(Some("foo"), Some("bar")).

[amis92]

@atifaziz

I believe that the major value of providing the single With method with optional parameters is that at the call site it's almost like an initializer/very readable.

record.With(name: "new name", phone: "123 456 789");

Anything that doesn't allow for a such a concise usage is not worth it for me. You can map to builders or use Update as well.

---

Personally I like how Optuple works, I'll consider it for my private code. But I don't want that kind of API for the optional With.

[amis92]

I do believe that making this configurable is not a very high cost, and with a reasonable default, even providing one's own implementation is a 7-liner:

    public struct Optional<T>
    {
        public Optional(T value) => (Value, HasValue) = (value, true);
        public bool HasValue { get; }
        public T Value { get; }
        public static implicit operator Optional<T>(T item) => new Optional<T>(item);
    }

[amis92]

Okay, I think I've been using Optional's previous versions, maybe 3.x - the current one doesn't offer the implicit "Some" interpretation of the T, so my desired API wouldn't work anyway.

(╯ರ ~ ರ）╯︵ ┻━┻

[atifaziz]

implicit "Some" interpretation of the T

Oh I see now how you were hoping to keep the call site looking clean, so yeah, record.With(name: Some("new name"), phone: Some("123 456 789")) could never compete with record.With(name: "new name", phone: "123 456 789").

[amis92]

Yup. But then I was wrong (at least about Optional package). Now I'm not sure what's the best approach.

Optuple

Using built in value tuples as you suggested is nice in terms of no additional code required, simple handling and overall less problems, except the API is, well, not the best it could be.

Custom Optional

Requiring some arbitrary API to be available, for example the suggested 7-liner (#102 (comment)), results in a nice API - but requires more work and more setup for the end-user. The result is expected to be a nicer API.

[bboyle1234]

I don't (yet) understand the disadvantages listed by @amis92

I'm strictly opposed to generating our own Optional type. It has a lot of issues:
Two projects with records will create two separate, incompatible types
It's incompatible with any other Optional implementation
It's arbitrary

We agree that this is what we want to see in the generated code and at the call site:

// generated code
public Person With(Optional<string> name = default, Optional<int> age = default) { 
  return new Person(name: name.GetValueOr(this.Name), age: age.GetValueOr(this.Age));
}

// calling code
var p = new Person("Tim", 21);
p = p.With(age: 22);

Would a good approach be to have a project like RecordGenerator.Core which contains the Optional class and is a dependency for all projects up the dependency chain? I know that takes us fairly close to using the https://www.nuget.org/packages/Optional package suggested earlier, but I think that package is massive bloat. Perhaps RecordGenerator.Core could be used to further advantage in the future as well?

This is the approach taken by the ImmutableObjectGraph project, which DOES have quite a lot of extra goodies to add to the core types package:

[amis92]

I'd definitely prefer the old conditional operator, and don't forget we always use Update :)

// generated code
public Person With(
    Optional<string> name = default,
    Optional<int> age = default)
{
    return Update(
        name.HasValue ? name.Value : this.Name,
        age.HasValue ? age.Value : this.Age);
}
// calling code
var p = new Person("Tim", 21);
p = p.With(age: 22);

[amis92]

Also I really don't enjoy the idea of imposing additional dependencies. This also results in downstream dependency versioning, which we'd have to take into account - right now we're very closed up, so whatever we break or change only affects directly the project it's used in. It's a good middle ground.

[bboyle1234]

Why do you use the 'Update' method? At first glance, it seems an entirely unnecessary "copy" of the constructor.

…

On Thu, Sep 26, 2019 at 9:21 AM Amadeusz Sadowski ***@***.***> wrote: Also I really don't enjoy the idea of imposing additional dependencies. This also results in downstream dependency versioning, which we'd have to take into account - right now we're very closed up, so whatever we break or change only affects directly the project it's used in. It's a good middle ground. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#102?email_source=notifications&email_token=AALA7ZUDDRGRHDMCDGY7PBTQLSZOJA5CNFSM4IYSB4B2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD7VRD7Q#issuecomment-535499262>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AALA7ZS3H7J4JOIPEQBLEBLQLSZOJANCNFSM4IYSB4BQ> .

-- Benjamin Boyle Skype: benjaminboyle

[amis92]

I've also came up with an even more complex approach. We'd generate a new nested class called Updater/Rebuilder/DeltaBuilder:

// written
public partial class Person
{
    public string Name { get; }
    public int Age { get; }
}
//generated
partial class Person
{
    public Person(string name, int age) => (Name, Age) = (name, age);

    public Person With(Action<Builder> build)
    {
        var delta = new DeltaBuilder(this);
        build(delta);
        return delta.ToImmutable();
    }

    public class Builder
    {
        public virtual string Name { get; set; }
        public virtual int Age { get; set; }
        public Person ToImmutable() => new Person(Name, Age);
    }

    private class DeltaBuilder : Builder
    {
        private readonly Person obj;
        private (bool set, string val) name;
        private (bool set, int val) age;
        public DeltaBuilder(Person obj) => this.obj = obj;
        public override string Name
        {
            get => name.set ? name.val : obj.Name;
            set => name = (true, value);
        }

        public override int Age
        {
            get => age.set ? age.val : obj.Age;
            set => age = (true, value);
        }
    }
}
// usage
person.With(x =>
{
    x.Age = 22;
    x.Name = "Joe";
});

Because why not. 🤷‍♂️

[amis92]

Why do you use the 'Update' method?

I think originally it was to support the inheritance, but then inheritance support was removed and Update stayed.

[atifaziz]

I've also came up with an even more complex approach. We'd generate a new nested class called Updater/Rebuilder/DeltaBuilder: …

Instead of DeltaBuilder, how about sort-of lenses? For example:

partial class Person
{
    public Person(string name, int age) => (Name, Age) = (name, age);
   
    public Person With(Action<Builder> builder)
    {
        var mutable = new Builder { Name = Name, Age = Age };
        builder(mutable);
        return mutable.ToImmutable();
    }
    
    public class Builder
    {
        public string Name { get; set; }
        public int Age { get; set; }
        public Person ToImmutable() => new Person(Name, Age);
    }

    public static Action<Builder> WithName(string value) => b => b.Name = value;
    public static Action<Builder> WithAge(int value) => b => b.Age = value;
}

Usage:

person = person.With(Person.WithAge(22) + Person.WithName("Joe"));

I'm using good old + to combine setters. 😉

Only trouble is that it's very allocate-y. Striking a good balance is the hardest. Nevertheless, an added benefit is that setters are disconnected so they can be cached and combined in interesting ways:

var joe = Person.WithName("Joe");
var age22 = Person.WithAge(22);

person = person.With(joe + age22);

var mods = joe;
mods += age22;
person = person.With(mods)

And if you introduce an overload:

    public Person With(params Action<Builder>[] builders) =>
        builders.Aggregate(new Builder { Name = Name, Age = Age },
                           (m, b) => { b(m); return m; },
                           m => m.ToImmutable());

then you can also do ad-hoc and dependent mutations:

person = person.With(joe, p => p.Age += 10);

It gives you a lot of expressive power, but as I said, it's allocate-y.

[amis92]

Yeah, the balance is hard to find.

For allocation-ignoring usages, just calling several With's isn't an issue:
person.WithAge(22).WithName("Joe");
It's clear, nice and all.

When allocations and copying matters (probably on huge records, or records with considerably-sized structs) and you're looking for a nice and concise API, the Update is obviously not good.

With all that said, I'm actually starting to think that the Optuple approach starts to look quite good. Some(...) is not too much to be added on user's own, I think. It's BCL-based, quite obvious in terms of API, and doesn't look too bad.

[atifaziz]

For allocation-ignoring usages, just calling several With's isn't an issue:
person.WithAge(22).WithName("Joe");
It's clear, nice and all.

When allocations and copying matters (probably on huge records, or records with considerably-sized structs) and you're looking for a nice and concise API, the Update is obviously not good.

Agree 100% but my point was that if you set allocations aside for a moment then you can get a lot more mileage and expressiveness out of With(Action<Builder> build) alone and providing a builder action static per property than needing something like DeltaBuilder and virtualisation of Builder. In fact, I'd be inclined to just call it with-do (WithDo(Action<Builder> builder)) or something along those lines because Action<> implies a side-effect.

Now if we want to address the issue of allocations then we need to get rid of the closure over the value to be set, and to make it vary with each property type, we will need to create some arbitrary N overloads of With (a reasonable choice for N being number of record properties):

// generated
partial class Person
{
    public Person(string name, int age) => (Name, Age) = (name, age);
   
    public Person With(Action<Builder> builder)
    {
        var mutable = new Builder { Name = Name, Age = Age };
        builder(mutable);
        return mutable.ToImmutable();
    }

    // overload #1

    public Person With<T>(Action<Builder, T> action, T value)
    {
        var builder = new Builder { Name = Name, Age = Age };
        action(builder, value);
        return builder.ToImmutable();
    }

    // overload #2

    public Person With<T1, T2>(Action<Builder, T1> action1, T1 value1,
                               Action<Builder, T2> action2, T2 value2)
    {
        var builder = new Builder { Name = Name, Age = Age };
        action1(builder, value1);
        action2(builder, value2);
        return builder.ToImmutable();
    }

    public class Builder
    {
        public string Name { get; set; }
        public int Age { get; set; }
        public Person ToImmutable() => new Person(Name, Age);
    }
    
    public static readonly Action<Builder, string> SetName = (b, v) => b.Name = v;
    public static readonly Action<Builder, int   > SetAge  = (b, v) => b.Age  = v;
}

With this approach, SetName and SetAge become statically cached instances and the values are passed separately to With, as in:

person = person.With(Person.SetName, "Joe", Person.SetAge, 22);

And since With is generic and types can be inferred, the order doesn't matter:

person = person.With(Person.SetAge, 22, Person.SetName, "Joe");

If we want dependency, then we can introduce further overloads that allow the values to be a function of the record:

public Person With<T>(Action<Builder, T> action, Func<Person, T> f)
{
    var builder = new Builder { Name = Name, Age = Age };
    action(builder, f(this));
    return builder.ToImmutable();
}

public Person With<T1, T2>(Action<Builder, T1> action1, Func<Person, T1> f1,
                           Action<Builder, T2> action2, Func<Person, T2> f2)
{
    var builder = new Builder { Name = Name, Age = Age };
    action1(builder, f1(this));
    action2(builder, f2(this));
    return builder.ToImmutable();
}

This permits dynamic expressions like “age the person by 10 years”:

person = person.With(Person.SetAge, p => p.Age + 10);

You can still mix with constants and those functions will be cached too:

person = person.With(Person.SetName, _ => "Joe", Person.SetAge, p => p.Age + 10);

This design has the advantage that With doesn't induce any allocations whatsoever and therefore can be used without much concern for cost. Moreover, each design or overload isn't mutually exclusive so you can entertain all choices:

• Wither that executes an action or an arbitrary set of actions that can be combined via +:
  • Person With<T>(Action<Builder, T> action, T value)
• Withers that execute a (sort-of) fixed set of actions, each given a distinct value:
  • Person With<T>(Action<Builder, T> action, T value)
  • Person With<T1, T2>(Action<Builder, T1> action1, T1 value1, Action<Builder, T2> action2, T2 value2)
• Withers that execute a (sort-of) fixed set of actions, each given a distinct dependent value:
  • Person With<T>(Action<Builder, T> action, Func<Person, T> f)
  • Person With<T1, T2>(Action<Builder, T1> action1, Func<Person, T1> f1, Action<Builder, T2> action2, Func<Person, T2> f2)

With all that said, I'm actually starting to think that the Optuple approach starts to look quite good. Some(...) is not too much to be added on user's own, I think. It's BCL-based, quite obvious in terms of API, and doesn't look too bad.

Hey, I'm just throwing ideas out to consider here and glad this one has strung a chord. Optuple certainly took away a lot of decision-anxiety in my projects about locking on a specific library or implementation of optional values. You just have squint hard and bend your mind a little to see the option or that there is no tuple/spoon. 😅

[amis92]

Yeah, lenses are a new concept I just recently discovered when browsing through https://github.com/louthy/language-ext

It's a cool concept but I'd say this actually deserves another issue completely. And I'd consider an Apply as the name for lens application method.