Skip to content

Allow the default value of a parameter to reference a final variable. #1541

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

Open
kasperpeulen opened this issue Mar 24, 2021 · 3 comments
Open
Labels
feature Proposed language feature that solves one or more problems

Comments

@kasperpeulen
Copy link

kasperpeulen commented Mar 24, 2021

A more general feature is suggested here:
#140

However, since there is no progress on this issue, I'm wondering if we could allow only a subset of non-constant values to be used as the default value of a parameter?

For example, could we allow final field and/or final variables to be used as a default value?

An example would be:

class Todo {
  final String body;
  final bool completed;
  final DateTime? dueDate;

  Todo({this.body, this.completed, this.dueDate});

  Todo copyWith({
    String body = this.body,
    bool completed = this.completed,
    DateTime? dueDate = this.dueDate,
  }) {
    return Todo(body: body, completed: completed, dueDate: dueDate);
  }
}

At this moment, making a nullable field final is almost like an anti-pattern in Dart.
You can not use the copyWith method anymore to set a field to null in an immutable way.
As such a copyWith method is generally implemented like this:

  Todo copyWith({
    String? body,
    bool? completed,
    DateTime? dueDate,
  }) {
    return Todo(
      body: body ?? this.body,
      completed: completed ?? this.completed,
      dueDate: dueDate ?? this.dueDate,
    );
  }

The only way left to change the dueDate to null is manually copying all the previous values:

  var todo = Todo(body: 'Code', completed: false, dueDate: DateTime.now());
  var sameTodo = todo.copyWith(dueDate: null);
  var newTodo = Todo(body: todo.body, completed: todo.completed, dueDate: null);
@kasperpeulen kasperpeulen added the feature Proposed language feature that solves one or more problems label Mar 24, 2021
@lrhn
Copy link
Member

lrhn commented Mar 24, 2021

I'd say no to this, with the same reasoning as for every other feature which works only for "final variables".
A Dart final variable is, and should be, indistinguishable from a getter. If the feature doesn't work for getters, then it isn't stable against refactorings which replace a final variable with a getter, something which should be a completely safe and non-breaking refactoring (if it isn't, it breaks much of the reason for having getters to begin with).

I'd probably be willing to make exceptions for final variables declared in the same class, or perhaps even the same library, because making that refactoring will immediately break the library you're looking at. It then becomes a class-internal or library-internal implementation detail that something is a final field.
That doesn't really work for instance variables, because a subclass can override the field with a getter. We don't have a non-virtual self.variable (like super.variable, only on the current class instead of the superclass), which could otherwise avoid that issue. We also don't have non-virtual or final declarations.

So, all in all, it does not appear like this would be viable.

For copyWith, I'd probably write it as:

Todo copyWith({
    String? body,
    bool? completed,
    DateTime? dueDate,
    bool clearDueDate = false,
  }) {
    return Todo(
      body: body ?? this.body,
      completed: completed ?? this.completed,
      dueDate:  dueDate ?? clearDueDate ? null : this.dueDate,
    );
  }

Not as convenient as other languages, but Dart doesn't have overloading allowing it to run different code depending on whether a parameter was omitted or not. All we have is default values, and if you can't create a sentinel value, there aren't many solutions left.

@kasperpeulen
Copy link
Author

kasperpeulen commented Mar 24, 2021

I think I leave my hopes at creating immutable values at Record's then 😅
Having to make an extra boolean in the copyWith method for every nullable field seems a bit too much,
although it is maybe the most straightforward solution that I have seen suggested, thanks for that,
it could also be autogenerated for every nullable field (an extra clear{field} bool).

With records, this would work right?

  typedef Todo = {String body, bool completed, DateTime? dueDate};
  Todo todo = (body: 'Code', completed: false, dueDate: DateTime.now());
  var newTodo = todo.with(dueDate: null);

@lrhn
Copy link
Member

lrhn commented Mar 24, 2021

I have absolutely no problem with allowing this.x as a default value (or any non-constant expression), we just don't do that now.
That'd be #429 (and a number of similar issues), with some design choices needing to be made, but even the most restrictive choices would still be OK with me, so I'm all for it.

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

2 participants