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

State.copyWith not ideal when it comes to nulls #97

Closed
MihaMarkic opened this issue Jan 22, 2019 · 9 comments
Closed

State.copyWith not ideal when it comes to nulls #97

MihaMarkic opened this issue Jan 22, 2019 · 9 comments

Comments

@MihaMarkic
Copy link

Hi guys, not strictly related to this library, but nonetheless important for redux concept.

The current samples shows state.copyWith something like:

class SomeState {
  final String text;

  SomeState copyWith({
    String text,
  }) {
    text: text ?? this.text
  }
}

Obviously this code won't work as expected when calling state.copyWith(text: null) where I'd expect that text property becomes null.
One way to solve passing nulls is to wrap values in a i.e. Nullable class:

class Nullable<T> {
  final T value;

  const Nullable(this.value);
  const Nullable.fromNull() : this.value = null;
}

and then call state.copyWith(Nullable.fromNull()) when setting text to null, or just state.copyWith(Nullable("some text")) when setting to a certain value.
The downside of this solution is that it creates more verbose code and it consumes more memory as everything is wrapped into a nullable.

Alternative solution would be to allow passing of additional bool properties, i.e.

SomeState copyWith({
    String text,
    bool setTextNull = false,
  }) {
    text: setTextNull ? null: (text ?? this.text),
  }

This might be better alternative, but it might be more confusing.
What do you guys think?

@brianegan
Copy link
Owner

It certainly isn't! For that type of thing, I almost always use the Optional class from Quiver. Option / Optional is a pretty common pattern for representing values that can be "present" or "absent." It's perfect for this case, since whoever is using the code knows "Ok, this value may or may not exist. It's optional!" They don't have to worry about setting two variables to change a single thing: text and setText.

https://pub.dartlang.org/packages/quiver

@MihaMarkic
Copy link
Author

@brianegan From what I understand, Optional it similar to my Nullable, right? I wish there were a cleaner way. For example, with C# I use similar approach, but C# lets you implement implicit cast operators. IOW you would still pass i.e. String, and copyWith with implicitly create Nullable(value) for you.

@brianegan
Copy link
Owner

brianegan commented Jan 22, 2019

Yep, it's pretty much the same thing, but has the extra information like optional.isPresent to check if the value exists -- and too bad Dart doesn't have that fancy implicit conversion magic! Overall, Nullable / Optional isn't perfect in Dart, but it's the best that I've used thus far.

@MihaMarkic
Copy link
Author

MihaMarkic commented Jan 22, 2019

Yeah, I agree. BTW, if you are curious about Param<T> c# implementation.
The only issue I have is that it bloats the code a bit, oh well, perhaps in the future...

@brianegan
Copy link
Owner

Hey @MihaMarkic -- can I provide any more help on this one? I'll close it for now since it's a bit stale, but please feel free to reopen!

@MihaMarkic
Copy link
Author

No, no further help needed, thanks.

@AAverin
Copy link

AAverin commented Mar 25, 2021

Is it resolved by dart nullability support? I still can't upgrade all my packages to check

@NaturalDevCR
Copy link

What about initialize the values and then compare?

Like this:

void main() {
  Vehicle myVehicle = new Vehicle(
    plate: "542541", 
    model: "Grand Vitara", 
    brand: "Suzuki", 
    year: 2021
  );
  
  print(myVehicle.year);
  myVehicle = myVehicle.copyWith(year: 2077);
  print("----------");
  print(myVehicle.year);
   
}

class Vehicle {
  
  final String plate;
  final String model;
  final String brand;
  final int year;

  Vehicle({this.plate = '', this.model = '', this.brand = '', this.year = 0});
  
  Vehicle copyWith({String plate = '', String model = '', String brand = '', int year = 0}) {
    return Vehicle(
      plate: plate == '' ? this.plate : plate,
      brand: brand == '' ? this.brand : brand,
      model: model == '' ? this.model : model,
      year: year == 0 ? this.year : year,
    );
  }
  
}

Basically Strings are initialized as empty strings, ints as zero for its initial value, bools could be false or true depending on the use case, and so on... at the end we'll simply need the ternary operator to make it work.

I'm a total noob of dart, so i'd ask... is this approach ok?

@jamesncl
Copy link

jamesncl commented Sep 28, 2021

@NaturalDevCR there are problems with these "sentinel values" if, for example, you ever want to set plate, model or brand to an empty string, or year to 0. Obviously you may never need to in your case, but as a general solution though I think you'll eventually get bitten. Also, in some cases there may be no obvious choice for a sentinel value (for example enums - what would you use then?)

I'm using the Optional package to work around the problem, so the code looks something like this:

final TZDateTime dateTime;
final double value;
final Duration? duration;

...

DataPoint _copyWith({
    TZDateTime? dateTime,
    double? value,
    Optional<Duration?>? duration})  {

    return DataPoint(
      dateTime ?? this.dateTime,
      value ?? this.value,
      duration: duration != null ?
        duration.orElseNull :
        this.duration,
    );
  }

In this example, duration is a nullable field, and the copyWith pattern works as normal. The only thing you have to do differently is if you are setting duration, wrap it in Optional like this:

Duration? newDuration = Duration(minutes: 60);
_copyWith(duration: Optional.ofNullable(newDuration));

Or if you want to set duration to null:

_copyWith(duration: Optional.empty());

Alternatively, there are code-gen solutions which handle this problem, because they provide automatically generated methods for copying new instances with changed values. For example, built_value and freezed

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

No branches or pull requests

5 participants