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

Problem: It is impossible to implement a Kotlin like copy method in Dart if you have "nullable" variables #137

Open
kasperpeulen opened this issue Dec 17, 2018 · 13 comments
Labels
feature Proposed language feature that solves one or more problems

Comments

@kasperpeulen
Copy link

kasperpeulen commented Dec 17, 2018

In Kotlin every data class is provided with a copy method. It copies the object and optionally it allows for setting new values. Here is an example:

import java.time.LocalDate

data class Todo(
    val body: String = "", 
    val completed: Boolean = false, 
    val dueDate: LocalDate? = null
);

fun main() {
    val todo = Todo(body = "Do this", dueDate = LocalDate.now().plusDays(1));
    // user decide to remove the dueDate
    print(todo.copy(body = "Investigate this some day", dueDate = null));   
}

The copy method can be implemented like this in Kotlin:

    fun copy(
        body: String = this.body, 
        completed: Boolean = this.completed, 
        dueDate: LocalDate? = this.dueDate
    ): Todo {
        return Todo(body, completed, dueDate);
    }

However, doing the same in Dart doesn't work, as in Dart default values of parameters must be compile time constant:

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

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

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

To allow for "default values" that are non-constant, often the following is recommended:

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

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

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

In many cases this works well. It's different in a subtle way, passing null has a special meaning now. It means, "give me the default value, whatever that is". This works well for "non-nullable" variables, as null can now be used to mean something different. But also for nullable variables if the default value is null (the two meanings of null overlap now).

However, it makes one case impossible, if you have a "true" nullable variable with a non-null default value. In this case, dueDate is truely nullable. null means here that there exists no dueDate for this todo. However, this makes it impossible to use the copy method to remove the due date of the todo:

main() {
  final todo = Todo(body: "Do this", dueDate: DateTime.now().add(Duration(days: 1)));
  print(todo.copy(body: "Investigate this some day", dueDate: null));
  // prints:
  // {
  //   "body": "Investigate this some day",
  //   "completed": false,
  //   "dueDate": "2018-12-18 11:11:11.607"
  // }
}

https://dartpad.dartlang.org/ccf1d10f1e5279ba93eef24c018b6290

In summary: It is impossible in Dart to give a "nullable" optional parameter a default non-constant value.

I think the most obvious way to solve this would be to allow optional parameters to have a non-constant default value. What is the reason for this restriction? I can not find any language with the same restrictions for the default values of parameters.

@eernstg eernstg added the feature Proposed language feature that solves one or more problems label Dec 17, 2018
@eernstg
Copy link
Member

eernstg commented Dec 17, 2018

I created #140 to express the general request that the default value mechanism should be less rigid than it is today (the default value must be constant). With that, this issue can be considered to be a response to #140, suggesting that we consider ideas from Kotlin as a starting point for a more expressive notion of default values.

We have discussed this topic earlier (allowing default values to be non-constant). Because of this, default values were not included when we defined constant contexts, so we are in a sense preparing for that kind of generalization.

See also dart-lang/sdk#25572 for some earlier discussions on a related topic.

@chriswiesner
Copy link

coming from Kotlin this is really cumbersome, i'd also appreciate to implement a simple copy mechanism

@munificent
Copy link
Member

What is the reason for this restriction?

I believe the original motivation is that it allows the compiler to check that an overriding method has the same default value as the method it overrides since the default values for both can be computed at compile time. Consider:

class A {
  int method([int n = 1]) => n;
}

void test(A a) {
  print(a.method());
}

A user who sees that code and looks up the declaration of method() on A probably assumes that since they are omitting the parameter, the value that it gets will be 1. But what they don't know is that their program is actually doing this:

class B extends A {
  int method([int n = 2]) => n;
}

main(List<String> args) {
  test(B());
}

The override in B has a different default. This is pretty confusing so Dart reports a warning if the default values don't match. It's not clear that this restriction actually carries its weight, though, and we have considered loosening this to allow non-const expressions there.

@rrousselGit
Copy link

rrousselGit commented Feb 8, 2020

We actually can do a proper copyWith by using default values.

There are two tricks available:

  • a combination of an abstract class + factory constructor, as showcased in a code-generator I released recently – see the generated file: https://github.com/rrousselGit/freezed

  • using FutureOr:

    class _ConstFuture<T> implements Future<T> {
      const _ConstFuture();
      @override
      dynamic noSuchMethod(Invocation invocation) {
        return super.noSuchMethod(invocation);
      }
    }
    
    class Example {
      int a;
      String b;
    
      Example copyWith({FutureOr<int> a = const _ConstFuture(), FutureOr<String> b = const _ConstFuture()}) {
        assert(a is int || a is _ConstFuture);
        assert(b is String || b is _ConstFuture);
        return Example()
          ..a = a is _ConstFuture ? this.a : a as int
          ..b = b is _ConstFuture ? this.b : b as String;
      }
    }

@rrousselGit
Copy link

Well, I may be biased but I feel that https://github.com/rrousselGit/freezed solves most problems in a very natural syntax.

The FutureOr variant is more of a work-around for all other situations that have a syntax different from freezed.

A FutureOr based copyWith could easily be generated as an extension method for example.

@eernstg
Copy link
Member

eernstg commented Feb 13, 2020

We have discussed the ability to access default values explicitly a number of times. One approach is to use null and let that stand for "please pass the default value" (but that clashes with other uses of null, that is, it doesn't work well for a parameter whose type is nullable).

The approach that I usually recommend is to have explicit syntax. With suitable defaults (no pun intended, of course). We could then make SomeClass.copyWith.default[0] denote the default value of the first positional parameter of copyWith as declared in SomeClass; we could omit SomeClass and copyWith when it is used in an invocation of copyWith on a receiver whose static type is SomeClass, and we could omit [0] when it's used in the first positional argument. We would then have this:

var y  = x.copyWith(map["a"] ?? default, map["b"] ?? default);

@rrousselGit
Copy link

A better solution, in my opinion, would be the if expression:

var value = x.copyWith(x: if (condition) value)

Which would be equivalent to:

var value = condition
  ? x.copyWith(x: value)
  : x.copyWith();

@eernstg
Copy link
Member

eernstg commented Feb 13, 2020

@tatumizer wrote:

"default" is based on the static type of x

Right, we have made it a warning (though not an error) to override such that a default value of a parameter changes, so that kind of override should indeed be rare. ;-)

Also, it's not exactly clear .. index .. named parameters

We could use the name to access the default value of a named parameter: default.a.

@idkq
Copy link
Contributor

idkq commented Feb 12, 2021

Solution:

class Dog{
  final String name;
  final int age;
  Dog({this.name, this.age});
  
  Dog.fromMap(Map<String, dynamic> map): 
    name = map['name'] as String, age = map['age'] as int;
  Map<String, dynamic> toMap() => {'name': name, 'age': age};
  
  Dog copyWith(Map<String, dynamic> fields){
    var newObj = this.toMap();
    for (final field in fields.entries)
      newObj[field.key] = field.value;
    return Dog.fromMap(newObj);
  }
}

void main() {
  Dog buddy = Dog(name: 'Buddy', age: 12);
  Dog puppy = buddy.copyWith({'age': null});
  
  // OUTPUT=> Name: Buddy Age: null
  print('Name: ${puppy.name} Age: ${puppy.age}');
}

@spkersten
Copy link

@idkq With that solution you lose type-safety. That seems like a higher cost than opting out of null safety.

@idkq
Copy link
Contributor

idkq commented Feb 12, 2021

@spkersten You can make it null safe and its better than using futures, other classes, etc IMHO

@lrhn
Copy link
Member

lrhn commented Feb 12, 2021

If you split your class into an interface and an implementation class, then you can have a default value that is not of the type of the original parameter (as @rrousselGit showed above):

abstract class NumBox {
  num? get value;
  NumBox(num? value) = _NumBox;
  NumBox replace({num? value});
}
class _NumBox implements NumBox {
  final num? value;
  _NumBox(this.value);
  NumBox replace({Object? value = "not a num"}) =>
    _NumBox((value is num?) ? value : this.value);
}

(not very useful with only the one property, but it shows the pattern).
If only you could do it in one class!

That's where it would be nice (read: my current favorite idea in this area) to allow a parameter to have both an external type and a different internal type, with the default value making the conversion:

class NumBox {
  final num? value;
  NumBox(this.value);
  NumBox replace({num? value as Object? = "not a num"}) =>
    NumBox((value is num?) ? value : this.value);
}

Here the replace parameter value has type num? in the interface, but internally in the function it is widened to Object?, and the default value can be taken from the wider type. (That would also allow {int x as int? = null}as a non-nullable value with anull` default value when omitted).

If it's something you can do if you split the class into an interface class and an implementation class, it seems reasonable if you could do that in one class declaration too, separating the interface signature from the implementation signature.

Alternatively, we can allow method forwarding like we do for factory constructors:

class NumBox {
  final num? value;
  NumBox(this.value);
  NumBox replace({num? value}) = _replace; // Forward call to `_replace`, missing arguments and all.
  NumBox _replace({Object? value = "not a num"}) =>
      NumBox((value is num?) ? value : this.value);
}

@jodinathan
Copy link

A better solution, in my opinion, would be the if expression:

var value = x.copyWith(x: if (condition) value)

Which would be equivalent to:

var value = condition
  ? x.copyWith(x: value)
  : x.copyWith();

I also thought of this feature, however, how would this work with more than one parameter?
ie

var value = x.copyWith(x: if (condition) value, y: if (otherCondition) otherValue)

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

9 participants