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

Allow setting named private properties #2005

Open
cedvdb opened this issue Dec 2, 2021 · 28 comments
Open

Allow setting named private properties #2005

cedvdb opened this issue Dec 2, 2021 · 28 comments
Labels
feature Proposed language feature that solves one or more problems

Comments

@cedvdb
Copy link

cedvdb commented Dec 2, 2021

I find dart constructors initializer list often hinder readability and needlessly redundant when trying to have private members. They are also sometimes annoying to read in public api scenarios

I would like the ability to populate private members with named parameters without resorting to an initializer list.

What I propose is to have those 2 snippets be equivalent:

Currently:

class House {
  final int _windows;  
  final int _bedrooms;
  final int _swimmingPools;
  House.named({
    required int windows,
    required int bedrooms,
    required int swimmingPools,
  })  : _windows = windows,
        _bedrooms = bedrooms,
        _swimmingPools = swimmingPools
;}

Suggared:

class House {
  final int _windows;  
  final int _bedrooms;
  final int _swimmingPools;
  House.named({
    required this._windows,
    required this._bedrooms,
    required this._swimmingPools,
  });
}

In both cases you'd call House.named(windows: 2, bedrooms: 1, swimmingPools: 4);. The first snippet has needless noise in it.

I'm sure this was a design decision but I can't find an issue other than the closed one I posted above about this.

@cedvdb cedvdb added the feature Proposed language feature that solves one or more problems label Dec 2, 2021
@mateusfccp
Copy link
Contributor

Although I agree with your reasoning, I find the proposed alternative too much obscure. It's not intuitive to have this._member as a parameter but have to use member: _ as argument.

@Levi-Lesches
Copy link

Levi-Lesches commented Dec 2, 2021

I don't mind the logic. It says that member is private, so even though you can pass it through the constructor like House.named(member: _), you shouldn't be able to access it by doing myHouse.member. It's essentially sugar for the current pattern of redirecting member to _member, which is so common it shouldn't really matter if it makes sense, since it would be recognizable anyway.

In general, there should be no guarantee that passing identifier to a constructor means that obj.identifier is valid, and this proposal is based on that.

@cedvdb
Copy link
Author

cedvdb commented Dec 2, 2021

Further more you can already do

class House {
  final int _windows;
  House(this._windows);
  House.alternatively(int windows) : _windows = window;
}

So there is somewhat of a parallel that will make the proposal feel intuitive. I can see how they both differ but they are also related

@lrhn
Copy link
Member

lrhn commented Dec 3, 2021

So, basically, if you use an initializing formal for a privately named field as a named parameter, the name of the parameter, in the function type of the constructor, becomes the name of the field without the leading _. It still initializes the private field.

This is non-breaking and new because you currently cannot have named parameters with private names, so there are no existing occurrences of {this._foo} anywhere.

We'd have to decide whether the final variable the initializing formal introduces into the constructor list should have the private name or the non-private name. Either is fine, we just have to make a choice.

We have to decide whether another positional parameter can have the same name as either the private name or the publicized name. (Probably whatever makes it not conflict with the final variable mentioned above, or both).

We have to decide how to handle edge cases like _, __, _2, or __foo. We can disallow all of them, that'd be perfectly reasonable.

I actually also have a problem with positional initializing formals, because they show up in dartdoc.
Maybe we should generalize it and make all privately named initializing formals count as their corresponding public name in the public API (and keep it private internally in the function, so as to not break existing code, for positionals, it only affects DartDoc).

That is:

  • For a private-named initializing formal, this._id,
  • where id is a valid non-private identifier,
  • the parameter name of the function is id instead of _id.
  • If id is not a valid non-private identifier, the name of the parameter is _id,
  • which is a compile-time error if the parameter is named.
  • The local final variable introduced by the declaration, bound to the passed value, is still named _id,
  • so existing code for positional initializing formals keeps working, only DartDoc changes.

It does introduce a new concept: The "corresponding non-private name" for a private name, which not all private names have.
And it uses that to invent names which do not occur in the source, which is ... slightly icky.

@jakemac53
Copy link
Contributor

It does introduce a new concept: The "corresponding non-private name" for a private name, which not all private names has.
And it uses that to invent names which do not occur in the source, which is ... slightly icky.

I don't actually agree this is a new concept. Everybody is already doing this, its just not codified in the spec, forcing us to manually spell it out each time :).

I have never heard of any other answer to this question other than removing the leading _ - because that is after all the thing that makes it private. It has to be removed. Other solutions like prepending something other than _ would be pretty bizarre imo.

According to the analysis here https://github.com/dart-lang/language/blob/db9f63185707c4c89a69118e842e4cc6e0e59cc3/resources/instance-initialization-analysis.md, >17% of all initializers are of this form (_field = field).

@leafpetersen
Copy link
Member

I have never heard of any other answer to this question other than removing the leading _ - because that is after all the thing that makes it private.

Of course you have, we were just discussing another answer to this question! :) As we were just discussing, we could just... not treat these as private in parameter lists. And then we're done, without any need for renaming schemes.

@jakemac53
Copy link
Contributor

(╯°□°)╯︵ ┻━┻

@cedvdb
Copy link
Author

cedvdb commented Dec 3, 2021

What is the rules behind this ?

class House {
  final int _windows;
  House(this._windows);
}

It just makes sens to me but I can't really explain a reasoning, it's just sugar for

class House {
  final int _windows;
  House(int windows) : _windows = window;
}

Isn't it kind of the same ?

Quoting the analysis above:

But it has to support both positional and named parameters.

Currently only positional is supported

@Levi-Lesches
Copy link

Levi-Lesches commented Dec 5, 2021

it's just sugar for

class House {
  final int _windows;
  House(int windows) : _windows = window;
}

Isn't it kind of the same ?

From my understanding of the proposal, that'd be correct. It's more annoying when there are multiple named private parameters, but that's the simple version.

@cedvdb
Copy link
Author

cedvdb commented Dec 6, 2021

You said would be but that snippet works today. It's not clear from the comment if you knew it or not.

@lrhn
Copy link
Member

lrhn commented Dec 6, 2021

The snippet works today, but the parameter name is the private identifier _windows. The name of positional parameters are not that important, but it does show up in the DartDoc, which is annoying, because the reader doesn't really need to know that the parameter is initializing a private field. That's an implementation detail leaking into the API (well, the API docs, but still visible).
Which means that it's not entirely equivalent to House(int windws) : _windows = windows;.

With the proposal here, the API docs would say the parameter name is windows. (That also means that the DartDoc should say [windows], not [_windows] when documenting the parameter, which is why it's not an entirely non-breaking change, name conflicts aside).

@munificent
Copy link
Member

I think this is a good idea. It definitely feels weird. If I had a time machine, I would sure as heck get rid of _ for privacy because it causes weird stuff like this. But without a time machine, we have to make the best with what we have.

The this. syntax is really popular. More than half of constructor parameters in a large corpus of pub packages use it. Named parameters are also popular—about as common as positional parameters. And we explicitly encourage users to make fields private when possible. So you have three very common, idiomatic features that can't be combined.

It's particularly frustrating because it's very obvious what the user wants to happen when they are combined. Fully 17% of field initializers in initializer lists are _foo = foo where a private field is initialized with a parameter of the corresponding non-private name.

There is (at least) one silly corner we need to worry about. If we say that this._id introduces a parameter named id then that would imply:

class C {
  var _a;
  var _b;

  C(this._a) : _b = a; // <-- Note "a", not "_a".
}

Is that what we want?

@cedvdb
Copy link
Author

cedvdb commented Dec 7, 2021

Is that what we want?

It looks like a has been created out of nowhere but is there an alternative ?

The following is valid today but does it make sens ? Irhn mentioned privacy is leaked in the documentation, which goes against the concept of privacy. It's not really privacy here, it's just the name of the param which happens to have a character reserved for privacy if I understood correctly, but the line is a bit blury and a bit brain busting.

class House {
  final int _windows;
  final int _doors;
  House.valid(this._windows): _doors = _windows;
  House.invalid(this._windows): _doors = windows;
}

If the behavior was the same for positionals, shouldn't valid and invalid be inverted ? (valid becomes invalid and vice versa). Both are imperfect anyway, might as well have one imperfect.

If I understand correctly those today are strictly equivalent:

class House {
  final int _windows;
  House.one(this._windows);
  House.two(int _windows): _windows = _windows;
}

I can't see a good reason to let a character that is also used as an access modifiers at the start of a parameter's name be valid but I'm going on a tangent a bit. It's just that treating it sometimes as a character and sometimes as an access modifier is well, context dependent which is harder on the brain.

@Levi-Lesches
Copy link

@munificent, @lrhn proposed earlier:

  • The local final variable introduced by the declaration, bound to the passed value, is still named _id.

That is, the parameter is passed in as id, but to the rest of the constructor (initializer list and body), it's still _id

@lrhn
Copy link
Member

lrhn commented Dec 7, 2021

Yes, the parameter name and the local variable name of the variable induced by the parameter need not be the same.
It's probably going to be confusing to some people if they're not the same, but I think it's reasonable and can be explained.

It would mean that the argument-to-parameter binding step and the parameter-to-local-variable binding step would be separate. Currently the two are conflated, with the "parameter" not really being used for anything (we bind argument values directly to local variables).

I think making that distinction is useful in other cases too:

  • External type of the parameter vs internal type of the variable. That's what covariant does - the static type matches the variable type, the runtime parameter type is Object. The throw happens when we try to bind the parameter value to the local variable. A similar functionality, like (Object? param as int) could do the same: External parameter type different from internal variable type, and a cast at the transition. We could even allow renaming void f(Object? param as int number) would have function type void Function(Object? param) and local variable int number, with a cast (if downcasting, for up-casting (int number as num numberOrDefault = double.nan) would allow you to extend the variable domain, and then only require the default value to be of the extended domain).
  • Default values. If we ever do "null is the same as omitting argument", then passing nothing or passing null would both bind the parameter to null, then the local variable would be bound to the default value if the parameter is null. Having two steps makes it clear.

We'd still have to use the public parameter name in DartDoc.
And I'm wary about introducing names that are not in the source, even if they are just removing a leading _.

@jakemac53
Copy link
Contributor

The fact that the "name" of a positional this. parameter today is the private name isn't really relevant because nobody has to care about the name outside of your package. It shows up in DartDoc which is unfortunate, but it never shows up in your users code.

If we don't remove the leading _ for named parameters I feel quite strongly that the feature will have negative overall value and we shouldn't do it at all. It will just become a code review headache, and we will end up with a lint rule that says to never use it.

I do think that coming up with some syntax (like the as <name> idea) which allows you to explicitly give it a public name might be a nice middle ground - it would be clear when looking at the constructor what name you should use without having to understand any implicit renaming, and it still removes a fair bit of boilerplate.

@cedvdb
Copy link
Author

cedvdb commented Dec 8, 2021

The constant redundancy that as would still bring outweights the small inconsistency here:

class C {
  var _a;
  var _b;

  C(this._a) : _b = a; // <-- Note "a", not "_a".
}

Because I suspect this would be used far less in comparison.

Beside it's not really inconsistent, if the feature is that it's just sugar for today's equivalent, it makes sens.

@MatrixDev
Copy link

@cedvdb , Dart already has too many inconsistencies, no need to bring more. Also what should happen if I have both - a and _a member?

@jakemac53
Copy link
Contributor

Also what should happen if I have both - a and _a member?

This would just be an error, you can't have two parameters with the same name. We already have that error so we shouldn't even need to add anything, although the analyzer/compilers may want a special message to help the user understand where the conflict came from.

@MatrixDev
Copy link

@jakemac53, but _a and a have different names and you can at this moment create both. Aka this code compiles:

class Test {
  final a = 0;
  final _a = 0;

  int test() => a + _a;
}

So you will need to change compiler and it will potentially break old code.

@jakemac53
Copy link
Contributor

The renaming here only happens for private this. constructor parameters. That is still potentially breaking (when it comes to positional parameters) but we can roll it out with language versioning so people don't get "broken" until they opt in.

@cedvdb
Copy link
Author

cedvdb commented Aug 9, 2022

Has anything been decided here ?

What about an optional as keyword, if the as is not provided it effectively is the same as providing the name of the private field without _. So the dartdoc can infer the right name without _ ?

class House {
  final bool _isBuilt;
  
  
  House.named({ required bool isBuilt}): _isBuilt = isBuilt;
  House.namedWithAs({this._isBuilt as isBuilt}); // suggar for the above
  House.namedWithoutAs({this._isBuilt}); // suggar for above
  
  House.namedOther({this._isBuilt as isAlreadyBuilt}); // not sure what the use case would be here
}

dart doc would show: [isBuilt] blablabla in all those 3 first cases. Maybe that's less obscure for some people although the result is the same ?

@munificent
Copy link
Member

Has anything been decided here ?

No decision yet. The language team is mostly focused on views, records, pattern matching, and macros, so other smaller language changes aren't getting much attention right now. That doesn't mean we've forgotten, it just means we try to focus on things in priority order.

@rrousselGit
Copy link

Personally, I don't like the idea. Because I would instead prefer if we could support private optional named parameters.
As in, optional parameters which can only be used by the defining library.

Such that we could do:

// file.dart
void fn({int? public, int? _private}) {}
fn(public: 42, _private: 21);

// another.dart
fn(public: 42); // _private cannot be used here

@cedvdb
Copy link
Author

cedvdb commented Jun 7, 2023

@rrousselGit

This seems more idiomatic, in dart's philosophy:

void fn({int? public}) => _fn(public: public);
void _fn({int? public, int? private}) {}

The alternative proposed would be confusing regarding the different meaning with constructors.

SomeConstructor(this._privateMember, { this._someMember }) {}  
doSomething({int? _privateParam })

// I assume the _privateMember would be usable outside but not _someMember ?

@rrousselGit
Copy link

This seems more idiomatic, in dart's philosophy:

void fn({int? public}) => _fn(public: public);
void _fn({int? public, int? private}) {}

Having to duplicate the function is exactly the sort of thing I'd want to avoid.

In the end, I think private named parameters and the shorthand for initializing private parameters kind of overlap in uses.
Private named parameters are common when considering things like copyWith where a class has private properties

I wish we could do:

abstract class Example {
  final int a;
  final int _private;

  // Allow cloning private vars too
  Example copyWith({int? a, int? _private});
}

The current best alternative is to do:

Example copyWith({int? a, @internal int private});

This is very similar in principle to this issue, where the private parameter had to somehow be made public.

@lrhn
Copy link
Member

lrhn commented Jun 8, 2023

I'd definitely go with:

  Example._(this.a, this._private);
  Example copyWith({int? a}) => _copyWith(a: a);
  Example _copyWith({int? a, int? private}) => Example(a ?? this.a, private ?? _private);

here too.

I design API for a living, to be used by other people than myself, so I care a lot more than just pragmatically about how an API looks and how it's presented.

If it's just your own class, and nobody else is ever going to use it (it's part if your application), you can do whatever you want with the API, it only affects yourself. (And in that case, maybe you wouldn't need to make _private private to begin with.)

I would never want a private name to be visible in the public API, because it's noise. A visible _ does nothing but potentially confuse readers, it has no upside. Even if it tells the reader that the thing is private, so they shouldn't worry about it, it's still better if they don't know about it at all.

I'm sure there is a middle-ground where an API is "public", but not really that important. Where strict separation isn't as necessary, and a pragmatic approach like Example copyWith({int? a, int? _private}) => ... would be reasonable. Heck, DartDoc could pretend the private parameter isn't there, so users won't need to know about something they can't use anyway.

But it has costs, some not obvious at the declaration point. You cannot implement the class interface in another library, or extend and override the method (say to add a public b field), because you cannot write the parameter name required by the function signature.

It can be used to leak a symbol of the private name. (Private symbols are not something you should depend on secrecy of for anything important, but still.)

It cannot be mocked, because you can't prepare for a call passing the private parameter, when(exampleMock.copyWith(a: any, _private: 0)).thenReturn(exampleMock); (or how it's done). This is not possible.
You can't even talk about the parameter in other libraries in any way.

The static type of the method includes the private name (the runtime type obviously does too). Unless we special case private parameters and remove inaccessible optional named parameters from the static type of members, but then it becomes even more weird if you cannot override the method without the parameter.
That may affect type inference, and propagate the private name into other function types where it's not expected. (Not sure it's likely for normal uses, but it's possible, so it'll eventually happen).

@lubritto
Copy link

lubritto commented Apr 22, 2024

I see some people avoiding using private named fields because of the current syntax, when you have more than 5 parameters it is weirdly bulky. I hope they give some attention to this issue in the future!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Proposed language feature that solves one or more problems
Projects
None yet
Development

No branches or pull requests

10 participants