Discussion: Constructor ergonomics

[daiplusplus]

I want to start a discussion about certain ergonomic issues I've encountered with C#'s constructors and if we could crowd-source possible solutions - or if there exist any workarounds today.

I note that the problem of the lack of primary constructors is solved with the Record Type functionality that looks to be introduced in C# 8.0, so this discussion will presume that language-feature is already implemented.

Logic before and between superclass constructor and own constructor

My first grievance concerns the inability to use local variables when invoking a superclass' constructor, consider:

class Foo {
    Foo(Int32 x, String y) {
        this.X = x + Math.Random();
        this.Y = y;
    }
    public Int32 X { get; }
    public String Y { get; }
}

class Bar : Foo {
    Bar(String packed)
        : base(
            ExtractX( packed ),
            ExtractY( packed )
        ) {
        this.OriginalX = ExtractX( packed );
    }
    private static Int32 ExtractX(String s) { ... }
    private static String ExtractY(String s) { ... }
    public Int32 OriginalX { get; }
}

Note how the subclass Bar uses a static method to convert its single String packed constructor parameter argument into separate arguments for use by Foo - which makes sense, except that Bar would like to use the value of the x parameter, but there is no way to pass intermediate values from the parent constructor call to the subclass, so the ExtractX method has to be invoked twice, which is inefficient (note that Foo.X does not store the original x value in this contrived example).

Today the main workaround to this problem is a static factory method with a "pass-through" primary-constructor (usually private or protected) - unfortunately any further subclasses cannot re-use the same factory method themselves, so any helper or parameter marshalling methods must also be exposed. But this approach isn't always possible if you're consuming a not-well-thought-out external library. The code below demonstrates this static factory-method approach:

class Bar2 : Foo {
    protected Bar2(Int32 x, String y)
        : base( x, y ) {
        this.OriginalX = x;
    }
    public Int32 OriginalX { get; }

    public static Bar2 Create(String packed) {
        Int32 x = ExtractX( packed );
        String y = ExtractY( packed );
        return new Bar2( x, y );
    }
    protected static Int32 ExtractX(String s) { ... }
    protected static String ExtractY(String s) { ... }
}

class Qux : Bar2 {
    public Qux(String packed)
        : base( /* has to call ExtractX and ExtractY again, even though Bar2 already does this in Bar2.Create, thus resulting in unwanted code-duplication */ )
    }
    // Additionally, if Bar2.ExtractX is private then Qux cannot be constructed unless it can initialize with the raw `x` and `y` values passed in by its consumers.
}

I don't want to make a formal proposal, but a possible general-solution to this problem would be to move the call to the base(...) constructor to inside the constructor body, and then allow code to run before base(...) is invoked, with the restriction that the code cannot use the this reference, like so:

class Bar3 : Foo {
    public Bar3(String packed) {
        // This code cannot use the `this` reference:
        Int32 x = ExtractX( packed );
        String y = ExtractY( packed );
        base( x, y );
        // Now that `base` is invoked, the superclass is initialized and `this` is now available:
        this.OriginalX = x;
    }
    protected Bar3(Int32 x, String y) {
        base( x, y );
    }
}

Boilerplate required to instantiate subclass of a superclass given a superclass instance

Briefly disregarding Record types, there exists an ergonomic problem in C# with copying data from a POCO class Foo to a derived POCO class Bar : Foo - as Bar's constructor needs to be redefined, with new parameters for each copied member, then the arguments passed to the superclass's constructor:

class Person {
    public String Name { get; }
    public DateTime DoB { get; }

    public Person(String name, DateTime dob) {
        this.Name = name;
        this.DoB = dob;
    }
}

class Student : Person {
    public DateTime EnrollmentDate { get; }
    public /* etc */

    public Student(String name, DateTime dob, DateTime enrollmentDate /* etc... */ )
        : base( name, dob, /* etc */ ) {
        this.EnrollmentDate = enrollmentDate;
    }
}

The ergonomic problem exists where you already have an existing Person instance with data but you want to promote it to a Student instance. Using normal primary-constructors you have to specify each property/argument which makes for harder maintenance as adding members to Person means updating code elsewhere in at least 3 different places.

A workaround exists by manually defining a "copy-constructor" that passes the properties values from an instance of the same type to a primary-constructor (or even the same constructor):

class Person {
    public String Name { get; }
    public DateTime DoB { get; }

    public Person(String name, DateTime dob) { // Primary constructor
        this.Name = name;
        this.DoB = dob;
    }

    public Person(Person p) // Copy-constructor
        : this( p.Name, p.DoB, /* etc */ )  {
    }
}

So then Student can be defined as:

class Student : Person {
    public Student(Person existing, DateTime enrollmentDate)
        : base( existing ) { // invoke the copy-constructor
        this.EnrollmentDate = enrollmentDate;
    }
}

...while this eliminates the maintenance requirement from the derived classes, the copy-constructor on Person must still be manually maintained. Looking over the Records proposal at https://github.com/dotnet/csharplang/blob/master/proposals/records.md I don't see any references to copy-constructors so I don't know if the work on Records types is considering this use case.

Dependency Injection Parameter Spam

I refer you to this comment by @Richiban: #1087 (comment) which details the problem of repetition in dependency-injection scenarios. None of the proposed solutions in the thread particularly appeal to me as they move declarations away from conceptually being class-level fields to being "special" parameters imbued with field persistence - this may confuse beginner users of the language but it also means the code is just less immediately-comprehensible.

Having to duplicate the type-name in the constructor method

(I'm sure this has been raised before) using the type-name as the constructor method name seems a bit strange to me: it makes the code brittle (because renaming a type won't rename the constructor without an IDE with refactoring functionality) and it adds no value. I imagine it was inherited from Java or C++, I'm sure we'd all prefer a more succint syntax like new(args) { } instead, additionally as new is already a keyword it means the constructors would be highlighted by the IDE without additional logic.

partial constructors and assigning to readonly fields outside a constructor

A constructor can call instance methods before the this instance is fully initialized - but readonly fields can only be set in the constructor. The C# compiler is not currently smart-enough to allow readonly fields to be set in a method invoked only from the constructor - while also not supporting the concept of partial constructors. In practice this is a problem when a class' constructor is toolchain-generated, which means either the constructor is tool-generated and the user cannot assign their own readonly fields, or the constructor is user-maintained and the tool-generated fields cannot be readonly because they're initialized in a separate method (such as the InitializeComponent method we're all familiar with from WinForms and WebForms).

Where to next?

All of these ergonomic issues makes me wonder if there's an alternative to constructors: what if, instead of a type declaring its own constructors _{(and that the type's consumers are able to invoke such constructors and that the type declares all the necessary constructors required (a primary constructor, a static factory method when parameter argument preprocessing logic is needed, convenience constructors or parameter-subset constructors, copy-constructors, etc)} instead the responsibility is offloaded to the compiler (that's its job after-all: to handle the boring repetitive tasks of generating code from a more abstract, higher-level representation): the class's "constructor" (at least its signature/prototype) is always generated by the compiler while any explicit constructor code is only written if it's actually needed - effectively a kind of "inferred constructor" generation - if that makes sense.

Additionally the C# compiler is well within its means to also generate the copy-constructors mentioned previously.

Just an idea off the top of my head. (Note this is not a proposal! Just my musing about how the ergonomics of C#'s constructors could be improved):

class Person {
    public String Name { get; }
    public DateTime DoB { get; }

    new( fields );
    // syntax for the compiler to create a constructor with a parameter mapped to each field in the class.

    new( this );
    // syntax for the compiler create a straightforward copy-constructor. If the class is a subclass of another then this invokes the copy-constructor for the superclass, if it exists, otherwise generate a compiler error.

    new( fields, String x, Int32 y) {
        this.Name = x;
    }
    // syntax where `fields` means that the parameters are created and the values copied, while any logic explicitly declared in the constructor body is run after the fields are set
}

What are your experiences?

In this discussion I'd like to see what ergonomic problems you've experienced with constructors - do you have any alternative solutions? Could we dispense with custom constructors entirely and rely on some variant of the Generic-factory-pattern with a generic constraint to allow subclasses to use a superclass's factory method? Could explicit dependency-injection be replaced with something more elegant (does anyone agree with merely injecting the service-locator instead of individual services?)

Or in short: "How can we make constructors better?"

Update

I also pitched-in some ideas for ctor "parameter-inheritance" in this thread: #4025 (comment)

[Unknown6656]

I absolutely love this proposal's idea to invoke the base constructor at a "later" point in a child's constructor code...

[HaloFour]

The C# compiler is not currently smart-enough to allow readonly fields to be set in a method invoked only from the constructor -

C# doesn't allow it because it's not valid according to the CLR. It produces unverifiable assemblies.

[daiplusplus]

That doesn’t mean we can’t make it valid, e.g. by having an annotation for methods that only ever assign to a field and never read from them (at least before assigning them), thusly it’s verifiable.

Not that verifiable assemblies have been popular. There’s loads of unsafe blocks in all the popular NuGet packages. And you can easily trip-up the CLR by type-punning in a struct with explicit field offsets, all without using unsafe too. Verifiability seems like it’s going the same way as CLSCompliant…

[KalleOlaviNiemitalo]

See also dotnet/runtime#43814 (comment) regarding maintenance of the IL verification rules.

[ebfortin]

I love your proposal of removing the need to use the type name as the constructor name. I just added an "issue" for discussion about this, before I read your comment.

I also like a lot the possibility of calling the base constructor anywhere in the constructor. I think Java allows it does it?

[HaloFour]

@ebfortin

I also like a lot the possibility of calling the base constructor anywhere in the constructor. I think Java allows it does it?

Java uses a function-like syntax to invoke the base constructor, but it is still restricted to being the first statement in the constructor. Both the JRE and the CLR severely restrict what you can do in a constructor prior to calling the base constructor to ensure that the instance is properly initialized so it doesn't make much sense to remove the limitations on the language.

[daiplusplus]

Yes, but there's no reason why locals cannot exist prior to calling the base ctor. That is what I'm asking for.

[carlreinke]

And indeed, with some contortions, local variables already can exist (via out) prior to calling the base constructor. (As-is, however, they are borderline useless.)

public class B
{
    public B(bool _, int x)
    {
        X = x;
    }
    
    public int X { get; }
}

public class C : B
{
    public C()
        : base(M(out int x), x)  // base constructor called with local variable
    {
    }
    
    private static bool M(out int x)
    {
        x = 123;
        return default;
    }
}

[theunrepentantgeek]

In the late 1990's, I used to do a lot of Delphi programming. The Object Pascal language required the implementer of any subclass to manually call the constructor of the base class, using the keyword inherited. It was common to place this as the first line of the constructor, but it could be placed anywhere.

This was a fertile source of bugs.

If memory serves, the usual case was a class that worked perfectly for the original scenario, but which failed spectacularly when reuse was attempted; because a different sequence of operations often meant that methods of the subclass ended up being invoked before initialization of the class was complete.

[daiplusplus]

What I’m proposing is that the this reference be verboten until after the call to base() returns - so this avoids the issue you’re referring to. I just want the ability to declare and use new locals before calling base().

[TonyValenti]

It is a sucky workaround but when I need to do something before the base() call, I'll use either overrides or a static method.

I hate to say it but I think VB got this one right.

[daiplusplus]

@HaloFour

Both the JRE and the CLR severely restrict what you can do in a constructor prior to calling the base constructor to ensure that the instance is properly initialized so it doesn't make much sense to remove the limitations on the language

To be clear, I am absolutely not calling for any changes to be made to that restriction at all - I'm asking for the ability to have a block of multi-statement "static-like" code, including support for local variables, that is executed between a subclass constructor and a superclass constructor. As the this reference isn't available there is zero possibility for bugs that might corrupt instance state.

Here's another example of why the current design is "broken" in my opinion. There exists an FxCop rule to require null-checking reference-type parameters of public methods and constructors, but if you require some kind of parameter argument marshalling between a subclass and a superclass then it becomes infeasible to comply with that FxCop rule.

public class BaseClass
{
    public BaseClass( Int32 x, String y )
    {
        // ...
    }
}

public class Derived : BaseClass
{
    public Derived( ComplexObject obj )
        : base( obj.X, obj.Y )
    {
        // ...
    } 
}

class ComplexObject
{
    public Int32 X { get; }
    public String Y { get; }
}

If obj == null then this will throw a NullReferenceException instead of ArgumentNullException.

The only way to comply with the rule is to do this (in C# 6.0):

public class Derived : BaseClass
{
    public Derived( ComplexObject obj )
        : base(
            obj?.X ?? throw new ArgumentNullException( nameof(obj) ),
            obj?.Y ?? throw new ArgumentNullException( nameof(obj) )
        )
    {
        // ...
    } 
}

...or this:

public class Derived : BaseClass
{
    public Derived( ComplexObject obj )
        : base( GetX( obj ), GetY( obj ) )
    {
        // ...
    } 

    private static Int32 GetX(ComplexObject obj)
    {
        if( obj == null ) throw new ArgumentNullException( nameof(obj) );
        return obj.X;
    }
    private static String GetY(ComplexObject obj)
    {
        if( obj == null ) throw new ArgumentNullException( nameof(obj) );
        return obj.Y;
    }
}

...and I'm sure you'll agree both options are patently ugly. And if GetX or GetY involve duplicate logic (e.g. to unpack values) then that's very inefficient because they can't share intermediate results.

[KalleOlaviNiemitalo]

Regarding computations and local variables before calling the base-class constructor, you can get a similar effect this way:

partial class Bar : Foo
{
    public Bar(String packed)
        : this(DoTheComputations(packed))
    {
    }

    private static (Int32 x, String y) DoTheComputations(string packed)
    {
        return (x: ExtractX(packed), y: ExtractY(packed));
    }

    private Bar((Int32 x, String y) arg)
        : base(arg.x, arg.y)
    {
        this.OriginalX = arg.x;
    }
}

Regarding field access before calling the base-class constructor, I see related restrictions in two clauses of ECMA-335 6th Edition:

• I.8.9.6.6 (Constructors), but that allows access to instance data that is not inherited, and in any case applies to CLS compliance rather than verification:

  CLS Rule 21: An object constructor shall call some instance constructor of its base class before any access occurs to inherited instance data. (This does not apply to value types, which need not have constructors.)

• III.1.8.1.4 (Class and object initialization rules), but that allows access to fields:

  No operations can be performed on an uninitialized this except for storing into and loading from the object's fields.

It seems the runtime would then allow a constructor to read and write fields (including backing fields of read-only auto-properties) of this before it calls a base-class constructor. Having that capability in C# would be another way to solve dotnet/runtime#38804 in EventListener construction.

[daiplusplus]

No part of my proposal/discussion involves setting fields in an instance prior to the base ctor completing, though I appreciate that that might have some use-cases (perhaps some provably safe cases where virtual methods can be invoked by by ctors).

[DavidArno]

Logic before and between superclass constructor and own constructor...

Your code example seems more Java-like than C#-like and this may be the cause of your confusion around how to handle such a situation. To solve your problem here, static factories and complex code aren't required. You simply need a private constructor that takes the extracted x & y:

public class Foo 
{
    public Foo(int x, string y) => (X, Y) = (new Random().Next(), y);

    public int X { get; }
    public string Y { get; }
}

public class Bar : Foo 
{
    public Bar(string packed) : this(ExtractX(packed), ExtractY(packed)) {}
    private Bar(int x, string y) : base(x, y) => OriginalX = x;

    public int OriginalX { get; }

    private static int ExtractX(string s) { ... }
    private static string ExtractY(string s) { ... }
}

---

Boilerplate required to instantiate subclass of a superclass given a superclass instance ...

The ergonomic problem exists where you already have an existing Person instance with data but you want to promote it to a Student instance...

A quote and a truism spring to mind when reading your code here:
“If the only tool you have is a hammer, you tend to see every problem as a nail.”, Abraham Maslow
"Favour composition over inheritance"

You have a person. You say you want a student. But what you really want is a student record associated with that person. Rather than disappear down a rabbit hole of trying to redefine how C#s inheritance model works, stop and question whether you took a wrong turn somewhere. Your solution effectively mangles composition in order to try and achieve inheritance. Therefore, use composition and all your complexity and need to change a fundamental feature of the language all fall away:

record Person(string Name, DateTime DoB);
record StudentRecord(Person Person, DateTime EnrollmentDate);

Job done: your data is modeled. Now you can focus on the business logic.