C#, Evil code

Backwards compatibility is (still) hard

53 Comments

At the moment, I’m spending a fair amount of time thinking about a new version of the C# API and codegen for Protocol Buffers, as well as other APIs for interacting with Google services. While that’s the context for this post, I want to make it very clear that this is still a personal post, and should in no way be taken to be “Google’s opinion” on anything. The underlying issue could apply in many other situations, but it’s easiest to describe a concrete scenario.

Context and current state: the builder pattern

The problem I’ve been trying to address is the relative pain of initializing a protobuf message. Protocol buffer messages are declared in a separate schema file (.proto) and then code is generated. The schema declares fields, each of which has a name, a type and a number associated with it. The generated message types are immutable, with builder classes associated with them. So for example, we might start off with a message like this:

message Person {
  string first_name = 1;
  string last_name = 3;
}

And construct a Person object in C# like this:

var person = new Person.Builder { FirstName = "Jon", LastName = "Skeet" }.Build();
// Now person.FirstName and person.LastName are readonly properties

That’s not awful, but it’s not the cleanest code in the world. We can make it slightly simpler using an implicit conversion from the builder type to the message type:

Person person = new Person.Builder { FirstName = "Jon", LastName = "Skeet" };

It’s still not really clean though. Let’s revisit why the builder pattern is useful:

  • We can specify just the properties we want.
  • By deferring the “build” step until after we’ve specified everything, we get mutability without building a lot of intermediate objects.

If only there were another language construct allowing that…

Optional parameters to the rescue!

If we provided a constructor with an optional parameter for each property, we can specify just what we want. So something like:

public Person(string firstName = null, string lastName = null)
...
var person = new Person(firstName: "Jon", lastName: "Skeet");

Hooray! That looks much nicer:

  • We can use var (if we want to) because there are no implicit conversions to confuse things.
  • We don’t need to mention a builder at all.
  • Every piece of text in the statement is something we want to express, and we only express it once.

That last point is a lovely place to be in terms of API design – while you still need to worry about naming, ordering and how the syntax fits into bigger expressions, you’ve achieved some sense of “as simple as possible, but no simpler”.

So, that’s all great – except for versioning.

Let’s just add a field at the end…

One of the aims of protocol buffers is to support an evolving schema. (The limitations are different for proto2 and proto3, but that’s a slightly different matter.) So what happens if we add a new field to the message?

message Person {
  string first_name = 1;
  string last_name = 3;
  string title = 4; // Mr, Mrs etc
}

Now we end up with the following constructor:

public Person(string firstName = null, string lastName = null, string title = null)

The code still compiles – but if we try to use run our old client code against the new version of the library, it will fail – because the method it refers to no longer exists. So we have source compatibility, but not binary compatibility.

Let’s just add a field in the middle…

You may have noticed that I don’t have a field with tag 2 – this is not an accident. Suppose we now add it, for the obvious middle_name field:

message Person {
  string first_name = 1;
  string middle_name = 2;
  string last_name = 3;
  string title = 4; // Mr, Mrs etc
}

Regenerate the code, and we end up with a constructor with 4 parameters:

public Person(
    string firstName = null,
    string middleName = null,
    string lastName = null,
    string title = null)

Just to be clear, this change is entirely fine in protocol buffers – while normally fields are assigned incrementally, it shouldn’t be a breaking change to add a new field “between” existing ones.

Let’s take a look at our client code again:

var person = new Person(firstName: "Jon", lastName: "Skeet");

Yup, that still works – we need to recompile, but we still end up with a Person with the right properties. But that’s not the only code we could have started with. Suppose we’d actually had:

var person = new Person("Jon", "Skeet");

Until this last change that would have been fine – even after we’d added the optional title parameter, the two arguments would still have mapped to firstName and lastName respectively.

Having added the middle_name field, however, the code would still compile with no errors or warnings, but the meaning of the second argument would have changed – it would now map onto the middleName parameter instead of lastName.

Basically, we’d like to stop this code (using positional arguments) from compiling in the first place.

Feature requests and a workaround

The two features we really want from C# here are:

  • Some way of asking the generated code to perform dynamic overload resolution at execution time… not based on dynamic values, but on the basis that the code we’re compiling against may have changed since we compiled. This resolution only needs to be performed once, on first execution (or class load, or whatever) as by the time we’re executing, everything is fixed (the parameter names and types, and the argument names and types). It could be efficient.
  • Some way of forcing any call sites to use named arguments for any optional parameters. (Even though in our case all the parameters are optional, I can easily imagine a case where there are a few required parameters and then the optional ones. Using positional arguments for those required parameters is fine.)

It’s hard (without forking Roslyn :) to implement these feature requests ourselves, but for the second one we can at least have a workaround. Consider the following struct:

public struct DoNotCallThisMethodWithPositionalArguments {}

… and now imagine our generated constructor had been :

public Person(
    DoNotCallThisMethodWithPositionalArguments ignoreMe =
        default(DoNotCallThisMethodWithPositionalArguments),
    string firstName = null,
    string middleName = null,
    string lastName = null,
    string title = null)

Now our constructor call using positional arguments will fail, because there’s no conversion from string to the crazily-named struct. The only “nice” way you can call it is to use named arguments, which is what we wanted. You could call it using positional arguments like this:

var person = new Person(
    new DoNotCallThisMethodWithPositionalArguments(),
    "Jon",
    "Skeet");

(or using default(...) like the constructor declaration) – but at this point the code looks broken, so it’s your own fault if you decide to use it.

The reason for making it a struct rather than a class is to avoid null being convertible to it. Annoyingly, it wouldn’t be hard to make a class that you could never create an actual instance of, but you can’t prevent anyone from creating a value of a struct. Basically, what we really want is a type such that there is no valid expression which is convertible to that type – but aside from static classes (which can’t be used as parameter types) I don’t know of any way of doing that. (I don’t know what would happen if you compiled the Person class using a non-static class as the first parameter, then made that class static and recompiled it. Confusion on the part of the C# compiler, I should think.)

Another option (as mentioned in the comments) is to have a “poor man’s” version of the compiler enforcement via a Roslyn Code Diagnostic – add an attribute to any method call where you want “All optional parameters must be specified with named arguments” to apply, and then make the code diagnostic complain if you disobey that. That diagnostic could ship with the Protocol Buffers NuGet package, which would make for a pretty nice experience. Not quite as good as a language feature though :)

Conclusion

Default parameters are a pain in terms of compatibility. For internal company code, it’s often reasonable to only care about source compatibility as you can recompile all calling code before deployment – but for open source projects, binary compatibility within the same major version is the norm.

How useful and common do I think these features would be? Probably not common enough to meet the bar – unless there’s encouragement within comments here, in which case I’m happy to file feature requests on GitHub, of course.

As it happens, I’m currently looking at radical changes to the C# implementation of Protocol Buffers, regretfully losing the immutability aspect due to it raising the barrier to entry. It’s not quite a done deal yet, but assuming that goes ahead, all of this will be mostly irrelevant – for Protocol Buffers. There are plenty of other places where code generation could be more robustly backward-compatible through judicious use of optional-but-please-use-named-arguments parameters though…

53 thoughts on “Backwards compatibility is (still) hard”

  1. A hypothetical “type such that there is no valid expression which is convertible to that type” could have no default argument, since they cause the implicit use of a constant expression at compile time.

    Liked by 2 people

    Reply

  2. Interesting post, and of course I couldn’t resist trying to solve the problem :)

    How about a fluent API? Properties can still be immutable; the actual constructor stays private so the public API can change without breaking anything. The main ugly thing is naming, since the method to set a property obviously can’t have the same name as the property.


    namespace PersonApp
    {
    public class Person
    {
    public Person()
    {
    }
    private Person(string firstName, string lastName)
    {
    FirstName = firstName;
    LastName = lastName;
    }
    public string FirstName { get; }
    public string LastName { get; }
    // Be nice if C# could cope with Properties and Methods having the same name…
    // Let's protest by violating naming conventions :)
    public Person firstName(string firstName)
    {
    return new Person(firstName, LastName);
    }
    public Person lastName(string lastName)
    {
    return new Person(FirstName, lastName);
    }
    public override string ToString()
    {
    return $"{FirstName} {LastName}";
    }
    }
    }

    view raw

    Person.cs

    hosted with ❤ by GitHub


    using System;
    namespace PersonApp
    {
    public class Program
    {
    public static void Main(string[] args)
    {
    var person = new Person().firstName("Jon").lastName("Skeet");
    Console.WriteLine(person.ToString());
    }
    }
    }

    view raw

    Program.cs

    hosted with ❤ by GitHub

    Like

    Reply

    1. That’s the kind of pattern I use in Noda Time, yes. It does end up with a lot of objects being created though – and while normally I don’t like micro-optimizing this sort of thing, setting all the properties on a message with N fields ends up using O(N^2) memory… I know the GC’s good, but it feels a little flamboyant. I suppose with the builder as well, clients could always use that when they’re concerned about the cost. I doubt that we’ll end up going this way, but it’s a good thought.

      Liked by 1 person

      Reply

      1. A fluent API returning a type that requires a finalizing call or an implicit conversion to the target type could be implemented via a builder to avoid creating an object for every property set. It would unfortunately create two objects when modifying a single property, though.

        Like

        Reply

        1. That’s what we’ve got in the current implementation, other than the implicit conversion. It’s still ugly, and an implicit conversion would also cause confusion when using var I suspect.

          It’s not so bad for more advanced developers who are used to the builder pattern, but it’s quite a burden to enforce on everyone wanting to use Protocol Buffers. It’s a matter of judging the right API for your target audience, and I’m hoping that GRPC will expand the target audience for Protocol Buffers a lot…

          Like

          Reply

      2. Wouldn’t it be best to fix this through a compiler or CLR optimization that reuses the ‘immutable’ object (since it should be able to detect that there are no future references to that object after the dot).

        Like

        Reply

        1. Not sure which “this” you’re referring to, but if you’re basically saying “Trust the GC/CLR to deal with garbage quickly” I’m happy to do so to some extent – but not when there can be hundreds of allocations, each involving copying hundreds of fields.

          Like

          Reply

          1. I mean the O(n^2) memory thing. And I’m not suggesting you just trust the GC/CLR – I’m suggesting that the .NET/C# devs add in an optimization for this specific use case.

            Like

            Reply

  4. As requested by via Twitter, a more detailed explanation:

    As far as I understand it, these feature requests are designed to improve backwards compatibility of APIs by improving how constructors handle optional parameters from client code consuming this API.

    However, C# itself is ALSO an API of sorts. If we change how C# handles constructor calls in the way your second request does and no longer allow unnamed arguments for optional parameters, that’s going to break a lot of legacy code, even very recent code.

    Suppose this was implemented in a future version of C# and you (Jon) decide to port the next major version of Nodatime over to it because of other new functionality being critical. Depending on the implementation of the C# change and how many optional parameters are called without a name, you could be looking at a long list of errors on the first attempt at building. You also then need to consider which constructors of the Nodatime API itself should be changed to make use of this new feature.

    In turn, anyone upgrading to the next version of Nodatime later on (because their own product is updating to take advantage of the new features in C#) will also need to change their own API calls that make use of the updated constructors.

    I realize that these changes likely make structural improvements to the readability and functionality of any code using these APIs, but many businesses also need to consider the implications for not just their own projects, but also for their clients and their users. If the necessary changes are too extreme, they might just not bother with this new feature, leading to a net result of C# being more complicated for no gain in code clarity and functionality.

    Like

    Reply

    1. I wasn’t suggesting changing the behaviour of any existing code. I was suggesting something like adding a modifier to a method which would enforce that calls specifying optional parameters do so by name. Adding that modifier to an existing method would indeed be an incompatible change, but then so do many other changes on existing code. That doesn’t mean it wouldn’t be useful for new code. (In the same way, if you have an existing API with many overloads to make up for the lack of optional parameters, converting those into a single method call with optional parameters breaks binary compatibility – but that doesn’t mean that optional parameters are useless, does it?)

      Like

      Reply

      1. I see. I was thinking more along the lines of a project-wide setting, but your solution indeed does make more sense. Something like a “named” modifier would indeed be useful. I also just realized that my example was inconsistent, with Nodatime itself having the project-wide setting configured, but the methods still requiring a flag. A modifier would be much more useful than a combination of the 2.

        Liked by 1 person

        Reply

  6. Isn’t your second feature request already possible (and a nice example for) using a roslyn analyzer?
    The analyzer could warn on or even prohibit using the API without named arguments.

    Like

    Reply

    1. Funnily enough, I was thinking about that just on the train this morning. Yes, it could – it would only be useful for VS2015 users of course, but better than nothing. Will edit to mention that.

      Like

      Reply
  7. Pingback: The Morning Brew - Chris Alcock » The Morning Brew #1874

  8. I actually like the mechanism StringBuilder uses: its Append-methods return the StringBuilder-instance itself, and the ToString-method creates the immutable string, like the current ProtoBuf builder implementation. I like the following kind of code for that:

    var person = Person
    .CreateBuilder()
    .SetFirstName(“Jon”)
    .SetLastName(“Skeet”)
    .Build();

    The advantage of the builder-pattern that the constructor-pattern does not have: it’s easy to build an instance gradually, like this:

    var builder = Person
    .CreateBuilder()
    .SetFirstName(“Jon”)
    .SetLastName(“Skeet”);

    if (isDoctor) { builder.SetTitle(“Dr”); }
    else if (gender == Gender.Male) { builder.SetTitle(“Mr”); }
    else if (gender == Gender.Female)
    {
    if (isMarried) { builder.SetTitle(“Mrs”); }
    else { builder.SetTitle(“Ms”); }
    }

    Like

    Reply

    1. Yes, it’s definitely useful to still have the builder – it’s just a pain to need it. Look at how much cruft is in your original code vs var person = new Person(firstName: "Jon", lastName: "Skeet"). Unless I am building something gradually, I don’t want to have to care about a builder – I just want to say what I want the end result to be.

      It’s not huge, and not awful to get used to – but it’s a barrier to entry.

      Like

      Reply

      1. I would argue that the two code samples are pretty similar, other than some simple coding preferences…

        I could just as easily write:
        var person = Person.Builder.withFirstName(“Jon”).withLastName(“Skeet”).Build();

        sure, the call to Build is an extra step (as mentioned first thing in your article), but I think it’s pretty minimal all said and done… but initializers/parameters on one line vs several is just preference (personally, I often place my named parameters in several lines for readability).

        I would also argue that the example above includes extra logic (dr/mr/mrs/ms) which would be just as messy for your approaches (perhaps cleaned slightly with ternary’s, but still not “ideal”)… though for those cases, I would actually argue in favor of those being calculated (readonly) properties anyway… the need to pass them in a proto buffer would only depend on whether you’d need those values on the other side, or whether they can be re-calculated after deserialization.

        Alternatively, what about a lambda style builder?
        var person = Person.Build(p => p.withFirstName(“Jon”).withLastName(“Skeet”));

        again, not entirely as intuitive as initializers, but addresses the issues.

        Like

        Reply

  9. Marc Gravell had a similar problem (binary incompatibility when using optional arguments) last year. http://blog.marcgravell.com/2014/08/optional-parameters-considered-harmful.html

    If you don’t add optional arguments in the middle, but only at the end, it is possible to remain binary compatible. Also this fix is only practical when you change the api by hand and not rely on code generation.

    In that case what you do is go from one method:
    Method(string firstName = null, string lastName = null)
    to two methods:
    Method(string firstName, string lastName)
    Method(string firstName = null, string lastName = null, string title = null)

    The first method will make the code binary compatible, the second method will be source compatible.

    Like

    Reply

  10. I think it is way worse to loose immutability than improve the construction of objects. There is nothing wrong with the builder pattern, in fact it is used for ImmutableCollections so is a known pattern used by Microsoft for key data structures, so please please do not make objects mutable, in fact please allow us to specify whether object can be value types as well. GC pressure is a big thing for apps that process a lot of data.

    Like

    Reply

    1. Optional arguments are a much worse solution to the problem than a builder pattern. Its just plain awful. It would be good if one could specify whether “null” is allowed for certain ref fields so guarantees can be made about all ref fields having non-null values.

      Like

      Reply

    2. I very much doubt that we’ll add an option to turn protobuf messages into structs instead of classes, especially as that would lead to a lot of boxing if we weren’t really careful.

      While I’m a big fan of immutability in general, and was perfectly happy for protobufs to use the builder approach previously, I think there’s a justifiable concern about it being too “different” for many developers. Yes, it’s far from unheard of – but I suspect the proportion of developers using ImmutableCollections is small too.

      I’ll take your feedback in mind, but I suspect we’ll still be going the mutable route – possibly with a “freeze” operation.

      Like

      Reply

  11. Ouch Jon, that freeze() operation really looks ugly to me.

    We solved this problem a few weeks ago by indeed using a fluent builder pattern. So in the end we achieve an immutable object and the builder genlty forces you to fill in the parameters.

    var person = Person.Create().WithFirstname(“Dwight”).WithLastname(“Matthys”).Build();

    Doesn’t look so akward to us, since using a constructor with named parameters also introduces some cruft in your code. The new can be easely associated with Create(), the With methods speak for themselves opposed to the named parameters and yes, we have one extra thingy at the end. But the benefits are greater than the losses.

    Just my 2 cents of course ;)

    Kind regards,
    Dwight

    Like

    Reply

    1. I think we’ll have to agree to disagree on this. In particular, I don’t see how “the with methods speak for themselves opposed to the named parameters” – while I agree that WithLastName(x) is clear, I’d say that lastName: x is at least as clear. What I like to do is look at a statement or expression and try to identify every identifier or value which is intrinsically part of what I’m trying to express – versus ones which are just formality. Additionally, using Person.Create() instead of new Person.Builder prevents the use of object initializers.

      Like

      Reply

  12. Here’s an idea you could try: make a Fody waiver which would substitute DoNotCallThisMethodWithPositionalArguments with System.Void. It’s perfectly legal in IL but C# won’t let you write default(System.Void) no matter how hard you try :-)

    Liked by 1 person

    Reply

  13. Here’s an idea: Make a Fody waiver that substitutes DoNotCallThisMethodWithPositionalArguments with System.Void. It’s perfectly legal in IL, but C# won’t let you write default(System.Void) no matter how hard you try :)

    Like

    Reply

      1. It’s wonderfully nasty, but I don’t think it would be practical – way to confuse callers :) I don’t know whether it would actually work from a CLR perspective, either.

        Like

        Reply

    1. I think you’ve missed the point of my post: a) it’s about limitations of optional parameters around compatibility; b) I’m implementing Protocol Buffers. I don’t think asking the GRPC project to switch to a Microsoft serialization format is going to fly…

      Like

      Reply

  16. How about a language request for a read-only enabled Object Initializer or maybe a Set-Once property?

    Like

    Reply

  17. Not sure if this is better at all but you could define a wrapper for each field which encodes its field position. This way you always know where the data is stored in old and new messages. Old code will not be disturbed by new fields and new code can use new fields wherever it wants to.

    class Person
{
    public string First { get; private set; }
    public string Middle { get; private set; }
    public string Last { get; private set; }

    public Person(FieldPos1<string> first, FieldPos3<string> last)
    {
        First = first.Value;
        Last = last.Value;
    }

    public Person(FieldPos1<string> first, FieldPos2<string> middle, FieldPos3<string> last)
    {
        First = first.Value;
        Middle = middle.Value;
        Last = last.Value;
    }
}

struct FieldPos1<T>
{
    public T Value { get; private set; }
    public FieldPos1(T value)
        : this()
    {
        Value = value;
    }
}

struct FieldPos2<T>
{
    public T Value { get; private set; }
    public FieldPos2(T value)
        : this()
    {
        Value = value;
    }
}

struct FieldPos3<T>
{
    public T Value { get; private set; }
    public FieldPos3(T value)
        : this()
    {
        Value = value;
    }
}

class Program
{
    static void Main(string[] args)
    {
        var p =     new Person(new FieldPos1<string>("Alois"), new FieldPos3<string>("Kraus"));
        var pV2 = new Person(new FieldPos1<string>("Alois"), new FieldPos2<string>("Christian"), new FieldPos3<string>("Kraus"));
    }
}

    You “only” need to ensure that you generate all optional overloads which leave out one or more fields because the did not exist yet to make it truly binary compatible. I am not sure if this positional wrapper is worth it. It is not as easy as using the person ctor with the real values but the concept it is quite easy to follow even for beginners which try to make their code compile by using Intellisense.

    Liked by 1 person

    Reply
  18. Pingback: Ideal Windows 10 App Development Environment

  19. Are protobufs only used with GRPC? And if so, what if you think about it from a procedure POV instead of from the message POV? That’s what the P in RPC stands for, after all. : )

    Like

    Reply

    1. No, protobufs can certainly be used for scenarios other than GRPC. The “P” part is indicated in the service definitions, not in the message definitions…

      Like

      Reply

  20. I think your proposed optional parameter syntax is bad idea irrespective of the C# limitations you point out.

    Most people interacting with your API won’t be experts; and even experts have better things to do than memorize a large API – and that means you want to avoid many “practical” but slightly different ways of achieving the same thing. The downside to the optional parameter approach is that it composes poorly. If you want to build a message with a few fields, you cannot delegate most of those fields to a helper method and then set the final field yourself – the construction call is indivisible; all fields must be set, or none.

    At the end of the day, what’s wrong with the new Person { FirstName = ... }.Build() syntax, or, if you prefer, a fluent api?

    If GC pressure is an issue with a fluent interface, you could always consider using value types.

    Like

    Reply

    1. Well, the constructor would probably have been an option rather than the only way of doing it – I’d probably have a builder as well, for those scenarios. I think it’s entirely reasonable for there to be multiple ways of achieving the same thing when they’re useful in different scenarios. As for what’s wrong with new Person.Builder {... }.Build() – the builder-specific parts are ugly, particularly if you’re only specifying a field or two. The language feature I’d really like is to be able to specify a builder type alongside the immutable type, so that the compiler would allow object-initializers to work with the builder and build the immutable type automatically… but that’s another matter. There are various other issues with using the builder pattern here that I don’t want to go into too much detail on – that wasn’t the point of the post, after all.

      As for the fluent interface with just an immutable type – O(n^2) space allocation and assignments (where n is the number of fields) is really nasty – and the places where it’s nasty are places where you don’t want a value type with all those fields, either. This is where the context is important – protocol buffers are defined by other people, and can have many, many fields… and the same codegen applies whether there are 2 fields or 100. In a tightly controlled API like Noda Time, the fluent syntax is absolutely fine, because I can make the right decision about what to do based on the context. For codegen of arbitrary protobufs, I don’t believe it’s the right answer.

      I’m still torn between the mutable API and the builder approach – both are ugly in different ways :(

      Like

      Reply

  21. What about having 2 versions of of each message class: mutable and immutable? You could implicitly convert mutable messages to immutable ones, and explicitly convert the other way round (.ToMutable()).
    You can then enable construction only for the mutable messages, which means they could serve as your builder, but could also be used as real messages. You can then nudge people toward preferring the immutable messages by naming the mutable messages MutablePerson and the immutable ones as Person which hint that they should be preferred.

    Like

    Reply
  22. Pingback: Automate the Planet

  23. Have you considered using a Dictionary<String, String> as your one and only parameter? That’s a mapping of parameter names to serialized objects your method needs to use, and then, you can simply just use what you need in your method from whatever the contents of this dictionary are, and/or serialize and de-serialize whatever you wish, manually, using XElement parsing.

    Like

    Reply

    1. No, I’d really rather not do that. Complete lack of compile-time assistance, requirement to convert everything to strings and back… nope, not for me :)

      Like

      Reply

      1. For compile-time assistance, you could define your own class that returns a parameter name and serialized object for a particular set of inputs. Your problem revolves around changing parameters anyways, so everything should work okay since you’ll change this construction logic with each build.

        Like

        Reply

  24. I sometimes use the following variant of the fluent pattern which, while more complex in some ways, helps eliminate some of the issues mentioned (e. g. excess copying):

    public class Person
{
    public Person(Action<Builder> builder)
    {
        if (builder == null) { throw ... }
        var personBuilder = new Builder(this);
        builder(personBuilder);
        // ensures that even if someone captures a reference to the
        // builder in the passed-in action they can't use it to further mutate this objkect
        personBuilder.End();
    }

    // all "immutable" properties allow private set
    public string FirstName { get; private set; }

    public class Builder
    {
        private Person person;

        // it's important that this isn't publicly accessible or else someone could use it to mutate a Person.
        // If we have our own assembly, internal will do just fine. However, if this source code will
        // go right in the consumer's assembly we're probably better off making our public Builder class
        // abstract and then providing a private implementation to forbid external construction
        internal Builder(Person person)
        {
            this.person = person;
        }

        public Builder FirstName(string firstName) 
        {
            this.person.FirstName = firstName;
            return this;
        }

        // other builder methods

        // comments about internal on the Builder constructor apply here too. If we had a private implementation
        // this could just be Dispose()
        internal void End() { this.person = null; }
    }
}

// we can then use this as
var person = new Person(p => p.FirstName("Jon"));

    Not as nice as optional parameters, but has the benefit of being fully backwards compatible (both source and binary) and being discoverable (the Person class is constructed through its constructor). As far as performance, we do no copying of fields since we write straight to the person object. On the other hand, we allocate the builder object and (likely) a delegate and closure object.

    Another thought I had when reading this is that I could see the library generating the optional parameters constructor as a nice-to-have on top of another more verbose pattern for any message types that have contiguously numbered fields from 0-N. That still leaves us with the binary backwards compatibility problem, but that can be easily solved by adding N constructor overloads.

    Like

    Reply

  25. I’m not sure why immutability is such a must have. These generated objects are purely used to serialise data over the wire, it’s not like anyone is using the generated classes as domain objects (if you are… oh dear). What’s the benefit of immutability in this scenario? Sure, it’s a nice a to have but not at the expense of additional GC pressure. In fact the uglier these generated objects are the better(!) as it would discourage people to reuse them where they shouldn’t (leaking outside their data layer).

    Like

    Reply

Leave a comment