Default non-constant value for factory parameter

[chikamichi]

Hi (again) 👋

I have this ValueObject implementing a [0.0; 1.0] double inclusive range:

import "package:freezed_annotation/freezed_annotation.dart";
import "package:modddels_annotation_fpdart/modddels_annotation_fpdart.dart";

part 'percentage.modddel.dart';
part 'percentage.freezed.dart';

@Modddel(
  validationSteps: [
    ValidationStep([Validation("range", FailureType<RangeFailure>())]),
  ],
)
class Percentage extends SingleValueObject<InvalidPercentage, ValidPercentage>
    with _$Percentage {
  Percentage._();

  factory Percentage(double value) {
    return _$Percentage._create(
      value: value,
    );
  }

  @override
  Option<RangeFailure> validateRange(percentage) {
    if (percentage.value < 0.0) {
      return some(const RangeFailure.min());
    }
    if (percentage.value > 1.0) {
      return some(const RangeFailure.max());
    }
    return none();
  }
}

@freezed
class RangeFailure extends ValueFailure with _$RangeFailure {
  const factory RangeFailure.min() = _Min;
  const factory RangeFailure.max() = _Max;
}

Which is leveraged in this kind of modddel factories:

class MapLayer extends SimpleEntity<InvalidMapLayer, ValidMapLayer>
    with _$MapLayer {
  MapLayer._();

  factory MapLayer.raster(
      {required MapLayerPosition position,
      required URI uri,
      @validParam bool loaded = false,
      @validParam bool enabled = false,
      Percentage opacity = 1.0}) {
    return _$MapLayer._createRaster(
      position: position,
      uri: uri,
      loaded: loaded,
      enabled: enabled,
      opacity: opacity,
    );
  }

  // etc.
}

The opacity attribute has an error: A value of type 'double' can’t be assigned to a variable of type 'Percentage'.

Should I somehow use Percentage(1.0) instead of the 1.0 literal? In which case I get the (expected) following error: The default value of an optional parameter must be constant.

[chikamichi]

Is it OK (Modddels-wise) going for:

  factory MapLayer.raster(
      {required MapLayerPosition position,
      required URI uri,
      @validParam bool loaded = false,
      @validParam bool enabled = false,
      Percentage? opacity}) {
    return _$MapLayer._createRaster(
      position: position,
      uri: uri,
      loaded: loaded,
      enabled: enabled,
      opacity: opacity ?? Percentage(1.0),
    );
  }

[CodingSoot]

Thanks for raising this issue ! I'll have a closer look and search for possible solutions.

[CodingSoot]

Unfortunately, this is not supported right now.

Is it OK (Modddels-wise) going for:

  factory MapLayer.raster(
      {required MapLayerPosition position,
      required URI uri,
      @validParam bool loaded = false,
      @validParam bool enabled = false,
      Percentage? opacity}) {
    return _$MapLayer._createRaster(
      position: position,
      uri: uri,
      loaded: loaded,
      enabled: enabled,
      opacity: opacity ?? Percentage(1.0),
    );
  }

I don't like this workaround because your Raster modddel will hold opacity as a nullable field, while it's not supposed to be nullable. There is no clean workaround for now, I'll need to add support for non-constant default values.

I'm thinking of implementing something like this :

factory MapLayer.raster({
  // ...
  @hasDefault Percentage? percentage,
}) {
  return _$MapLayer._createRaster(
    // ...
    /// This parameter is non-nullable
    percentage: percentage ?? Percentage(1.0),
  );
}

The percentage parameter of the _create method will be non-nullable, so you're forced to pass a default value instead of null. The @hasDefault annotation will basically mean "this parameter is not really nullable, null will be replaced by a default value".

This is the simplest solution I could come up with. It would allow you to add any defauIt non-constant values (even for dependency parameters for example). I'll see if I can come up with something better before implementing this.

[CodingSoot]

Okay, the @hasDefault solution is fairly easy to implement. Only problem is how that would translate to the generated ModddelParams class.

Usually, the generated ModddelParams has the same constructor as your modddel (or case-modddel).

For example, if you have this modddel :

@Modddel(
  // ...
  generateTestClasses: true,
)
class MyClass extends MultiValueObject<InvalidMyClass, ValidMyClass>
    with _$MyClass {
  MyClass._();

  factory MyClass({
    int param1 = 18,
    required SomeClass param2,
  }) {
    return _$MyClass._create(
      param1: param1,
      param2: param2,
    );
  }

  //...
}

The generated ModddelParams class looks like this :

class MyClassParams extends ModddelParams<MyClass> {
  const MyClassParams({this.param1 = 18, required this.param2}) // ...
  //....
  final int param1;

  final SomeClass param2;
}

But what if we had a non-constant default value for param2 ?

  factory MyClass({
    int param1 = 18,
    @hasDefault SomeClass? param2,
  }) {
    return _$MyClass._create(
      param1: param1,
      param2: param2 ?? SomeClass(0),
    );
  }

With this approach, I don't see how MyClassParams can possibly have the same non-constant default value for param2. So we'll need to make param2 in MyClassParams required and non-nullable, which is not ideal.

[CodingSoot]

Upon reflection, I think we should stick with the rule of the ModddelParams class having the same constructor as the factory constructor of the modddel.

In our example, this means that in MyClassParams, param2 should be optional and nullable just like in the factory constructor of MyClass :

class MyClassParams extends ModddelParams<MyClass> {
  const MyClassParams({this.param1 = 18, this.param2}) // ...
  //....
  final int param1;

  final SomeClass? param2;
}

And you'll use it like this in tests :

group('my tests', () {
  final testMyClass = TestMyClass();

  testMyClass.isSanitized(
    MyClassParams(), // Same as MyClassParams(param2: null)
    sanitizedParams: MyClassParams(param2: SomeClass(0)),
  );
});

In this test, we verify that when MyClass is created without providing param2 (implying param2 is null), the resulting modddel's param2 property will equal SomeClass(0).

All in all, the rationale behind this approach is quite clear when we consider that replacing null with a default value can be seen as a form of sanitization, especially since it happens inside the factory constructor.

[CodingSoot]

Last thing to figure out is the name of the annotation 😅 : Should we go with @hasDefault or something more explicit like @defaultIfNull ?