dart-lang / language

Design of the Dart language
Other
2.65k stars 201 forks source link

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

Open kasperpeulen opened 3 years ago

kasperpeulen commented 3 years ago

A more general feature is suggested here: https://github.com/dart-lang/language/issues/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);
lrhn commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 https://github.com/dart-lang/language/issues/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.