Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Proposal]: Direct constructor parameters #4024

Open
1 of 4 tasks
MadsTorgersen opened this issue Oct 16, 2020 · 14 comments
Open
1 of 4 tasks

[Proposal]: Direct constructor parameters #4024

MadsTorgersen opened this issue Oct 16, 2020 · 14 comments
Assignees
Milestone

Comments

@MadsTorgersen
Copy link
Contributor

MadsTorgersen commented Oct 16, 2020

Direct constructor parameters

  • Proposed
  • Prototype: Not Started
  • Implementation: Not Started
  • Specification: Not Started

This proposal is extracted from #3137.

Summary

Allow constructor parameter lists (including those of primary constructors #2691) to directly list instance fields and properties of the enclosing class in lieu of a parameter declaration, meaning that a parameter with the same name and type as the member will be declared, and it's value assigned to the member during construction.

Motivation

Constructors usually require a lot of boilerplate to a) declare a parameter for the initial value of a given member, and b) assigning it to the member. Oftentimes these trivial assignments make up the only code or the majority of code in a constructor body; e.g.:

public class Point
{
    public int X { get; }
    public int Y { get; }
    
    public Point(int x, int y) 
    { 
        this.X = x;
        this.Y = y;
    }
}

This feature would make such constructor parameters shorter (they wouldn't need to repeat the type) and eliminate the assignment code entirely, allowing the above to be written simply as:

public class Point
{
    public int X { get; }
    public int Y { get; }
    
    public Point(X, Y) { }
}

Detailed design

Allow a constructor parameter list to directly mention property and field members (direct and inherited) to be initialized in lieu of a parameter declaration. As a result, a parameter would implicitly be declared of the same name and type as the member, and the initialization would happen at the beginning of the constructor body, in the order of appearance.

The example

public class Point
{
    public int X { get; }
    public int Y { get; }
    
    public Point(X, Y) 
    { 
        // ... validation 
    }
}

Would generate:

public class Point
{
    public int X { get; }
    public int Y { get; }
    
    public Point(int X, int Y) 
    { 
        this.X = X;
        this.Y = Y;
        // ... validation 
    }
}

In order to avoid repeated initialization when there are multiple constructors, or in the face of inheritance, we would not generate initialization of a field or property whenever the corresponding parameter is passed to this(...) or base(...):

public abstract class Person
{
    public string Name { get; }
    public Person(Name) { } // Name is initialized
}
public class Student : Person
{
    public int ID { get; }
    public Student(Name, ID) : base(Name) { } // Only ID is directly initialized here
}

Drawbacks

  • The generated parameter names would violate convention by starting with an upper-case letter. We could try to get smart about this (if the field or property name is classified as an upper-case letter, substitute for the corresponding lower-case letter in the generated parameter name), but it seems we have already accepted similar behavior in the primary constructors on records in C# 9.0.
  • The syntax of having a single identifier in parameter position will be "used up" by this feature. We couldn't easily use it for some other feature later.

Alternatives

None known.

Unresolved questions

There is a scoping subtlety where this feature is used in conjunction with primary constructors, as in:

public class Point(X, Y)
{
    public int X { get; }
    public int Y { get; }
}

Here, the constructor parameter list occurs outside of the class body, yet it references members that are inside. How does this gel with the fact that C# 9.0 primary constructors in records do not have e.g. nested types directly in scope?

Design meetings

https://github.com/dotnet/csharplang/blob/master/meetings/2020/LDM-2020-11-11.md#direct-constructor-parameters
https://github.com/dotnet/csharplang/blob/main/meetings/2022/LDM-2022-09-28.md#ungrouped

@CyrusNajmabadi
Copy link
Member

The generated parameter names would violate convention by starting with an upper-case letter. We could try to get smart about this (if the field or property name is classified as an upper-case letter, substitute for the corresponding lower-case letter in the generated parameter name), but it seems we have already accepted similar behavior in the primary constructors on records in C# 9.0.

The real drawback of this is that we effectively can't migrate people from any existing code to this pattern. That seems very unfortunate. While i don't love it, if we're not going to auto-name these, could we have something new like public Point(X:x, Y:y) or public Point(x:X, y:Y) that indicates: "name the parameter this, and connect it to that"?

Then everyone who has shipped code that could benefit from this could move their code without it being a breaking change.

@alrz
Copy link
Member

alrz commented Oct 17, 2020

I think the same mechanism can be used to simplify custom Deconstruct and probably pattern-based With?

@qrli
Copy link

qrli commented Oct 17, 2020

To me, this feature is not that important for normal class. However, it could addresses a major fatigue for classes with dependency injection, which is very common in ASP.NET Core, etc.

public class FooService
{
    private readonly IBarService barService;
    private readonly IBazService bazService;
    
    public FooService(barService, bazService) 
    { 
        // ... 
    }
}

In this case, it is typically not property but simply private readonly field.

Regarding "use up the single identifier" syntax, I'm fine with additional token in case needed:

    public FooService(for barService, for bazService) 

or

    public FooService(=barService, =bazService) 

@alrz
Copy link
Member

alrz commented Oct 17, 2020

I think classes with dependencies are best served with primary constructors?

class FooService(IService service)

That would probably trim half of any aspnet app.

I think this feature would be only useful in very specific scenarios, like multiple constructors etc. Other than that, primary constructors would cover most of the use cases out there.

@qrli
Copy link

qrli commented Oct 17, 2020

@alrz Primary constructor is more designed for small classes, while service classes typically have relative large body.

  • The parameter list (injected services) is typically > 3 and can be many, e.g. 10. And service interface/class names are usually long. The result is that the parameter list is typically written as one parameter per line. In such case, I don't feel primary constructor syntax looks nice.
  • For service classes, it is relative common to have more than simple assignment in constructor, so a constructor body is needed anyway. This makes primary constructor somewhat a duplication.
  • Primary constructor generates public properties instead of private fields.
  • The service fields are readonly. As what I know, current primary constructor does not imply readonly, or it needs more annotations. Likewise, the field is also not always private.

@alrz
Copy link
Member

alrz commented Oct 17, 2020

The parameter list (injected services) is typically > 3 and can be many

That's exactly why using primary constructors is a win since you don't need to write all the field declarations.
In this proposal you will have auto assignments, but you still need to write field decls and "parameters" in the form of C(field)
Of course "looking nice" when you break them in multiple lines is a personal preference.

Primary constructor generates public properties instead of private fields.

primary constructors don't generate anything public on regular classes. so under the hood you most likely have fields.

The service fields are readonly

As mentioned in #4025 you can make them readonly but I'd argue they should be readonly by default.

For service classes, it is relative common to have more than simple assignment in constructor, so a constructor body is needed anyway

That's what primary constructor body is for, so you won't lose that functionality.

@qrli
Copy link

qrli commented Oct 18, 2020

If the generic primary constructor design can fill this gap, it is fine to me.

That being said, I'd say, if primary constructor is designed to be flexible enough to solve 90% of the problems, it will remove most value of "Direct constructor parameters", as it will only introduce two similar ways to do the same thing. I see this proposal as an complimentary solution to avoid primary constructor from becoming a full class declaration mechanism.

@BhaaLseN
Copy link

Allowing fields and not just properties could get rid of a lot of boilerplate (ignoring validation for a second).

But does this also work with additional parameters that are not properties/fields? Like (useless example, but you get the idea):

public class Point
{
  public int X { get; }
  public int Y { get; }

  public Point(X, Y, int scale)
  {
    if (scale != 0)
    {
      X *= scale;
      Y *= scale;
    }
  }
}

@laicasaane
Copy link

After making a small library with a handful of structs, I'm totally agree with this proposal. I want to reduce boilerplate code as much as possible for structs.

@333fred 333fred added this to the Working Set milestone Nov 11, 2020
@ILMTitan
Copy link

public Student(Name, ID) : base(Name) { } // Only ID is directly initialized here

This part of the proposal seems confusing and not very valuable to me. Is Name not initialized only if base also uses this syntax? If not, then what if Name is not initialized to the first parameter of the base constructor?

public abstract class Person
{
    public string Name { get; }
    public Person(Name) { } // Name is initialized
}
public class Student : Person
{
    public int ID { get; }
    public string PreferredName {get;}
    public Student(PreferredName, ID) : base(PreferredName) { } // Is PreferredName initialized here?
    public Student(Name, PreferredName, Id): base(Name) { }
}

It is that much of a hassle to type out the type name of a parameter that is not directly initialized? It seems like it would resolve most of the ambiguities.

    public Student(string Name, PreferredName, Id): base(Name) { } // Name clearly not initialized here.

@zlepper
Copy link

zlepper commented Nov 16, 2020

Having just messed around with the new record types, i'm sorely missing this for classes now, especially around DI, where you keep repeating the same things over and over.

However I would like to take it a bit further than the original proposal. I would get completely rid of the fields, and do the same thing records do, so you could do the following:

public class MyService(private ILogger<MyService> _logger, private IMyOtherService _otherService) {
    public void DoStuff() {
      _logger.LogInfo("Doing thing");
      // ...
   }
} 

Using the mentioned syntax would mean there is no discussion around the parameter names, they just become the field names also. It has the additional benefit that the syntax looks a lot like records, so there isn't a need to remember two separate syntaxes, for doing practically the same things.

It is possible to do this in Kotlin, and it saves so much boilerplate it's rather crazy.

@MikaelElkiaer
Copy link

I like how it works in Typescript with "Parameter Properties":
https://www.typescriptlang.org/docs/handbook/2/classes.html#parameter-properties

@navidzehi
Copy link

In addition to all comments I suggest using "this" to access property and even have a constructor without body:

// ctor
 public Person (this.FirstName, this.LastName);

@333fred 333fred modified the milestones: Working Set, Likely Never Sep 28, 2022
@333fred
Copy link
Member

333fred commented Sep 28, 2022

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests