Value.ofNullable has a wrong assertion

[ariefwijaya]

Describe the bug

it says that Value<T>.ofNullable constructor is to Create a value that is absent if value is null and [present] if it's not. and also The functionality is equiavalent to the following: x != null ? Value(x) : Value.absent()

But the problem is the assertion blocked the functionality:

  const Value.ofNullable(T? value)
      : assert(
          value != null || null is! T,
          "Value.ofNullable(null) can't be used for a nullable T, since the "
          'null value could be both absent and present.',
        ),
        _value = value,
        present = value != null;

Why Value.ofNullable(null) can't be used for a nullable T, ?? This assertion should be removed so we can use this construction to the equivalent functionality x != null ? Value(x) : Value.absent()

[simolus3]

The point of the Value class is to distinguish between two common cases that one might intuitively represent as null:

1. Null as a present value: The column should explicitly be set to NULL in SQL.
2. Null as an absent value: The column should use the default value for inserts, or be unchanged for updates.

In other words, a Value stores more information than a nullable Dart expression. When we know that a column can only store non-nullable values (for instance because it has a NOT NULL constraint), the first case isn't relevant and using a nullable Dart object achieves the same thing as a value. Value.ofNullable exists only for this scenario, and this is documented as well. So it is intentional that ofNullable only works for non-nullable T.

If you have nullable columns and you want to interpret null as absent instead of setting the column to null, you'll have to spell out x != null ? Value(x) : Value.absent(). The intention here is that this choice is not obvious, and spelling it out makes it clear in code that one of the two null interpretations has explicitly been chosen. So I'll close this as working as intended.

[praxder]

I also find this constructor name very confusing. It's silly that a constructor named ofNullable, that takes a nullable value, actually is implemented in such a way that the parameter shouldn't be null. Very confusing. Especially given the comment

  /// The functionality is equiavalent to the following:
  /// `x != null ? Value(x) : Value.absent()`.

[simolus3]

The documentation also points out that the constructor should only be used when T is non-nullable though, which is also what the assertion is checking.

actually is implemented in such a way that the parameter shouldn't be null

The parameter can be null just fine, the type parameter is the problem. You can't do Value<String?>.ofNullable(null) since we then again have the ambiguitiy of not knowing "which null" you've meant.

[jvivasb20]

I agree with @praxder. When would this be applicable? For instance, if I'm certain that the value I'm inserting isn't nullable, I would simply use Value() instead of Value.ofNullable().

[simolus3]

It's applicable when you have a non-nullable column which you may or may not want to update. Then you can use Value.ofNullable(myNullableString) in a companion for updates, which would update the column if myNullableString is non-null and leave it as-is otherwise.

[jvivasb20]

Alright, I understand now. Thank you for clarifying.

[jvivasb20]

Are you considering adding a feature to handle nullable values? For instance, something like a wrapper for this:

String? polygonId;

final newSublot = SublotsCompanion(
name: drift.Value(controllerSublotName.text),
farmId: drift.Value(farmId),
cropId: drift.Value(currentCrop.value!.id),
polygonId: polygonId != null ? drift.Value(polygonId) : const drift.Value.absent(), // This comparison
);

await _farmsUseCase.insertSublot(newSublot);

[simolus3]

Yeah, I think I'll add a Value.absentIfNull constructor for this. Given that the name is explicit about what happens with null, I think that could work for all types.