binder icon indicating copy to clipboard operation
binder copied to clipboard

`Scope.write()` without explicit type argument is not compile time safe

Open hcbpassos opened this issue 4 years ago • 5 comments

When there's no explicit type argument on Scope.write(), it is compile time valid to pass a state that can't be attributed to ref. Take this example:

final countRef = StateRef(0);

class CounterLogic with Logic {
  ...
  void increment() {
    write(countRef, 'banana'); // compiler: yup, go ahead.
  }
}

This happens because, implicitly, T becomes the least upper bound type of both arguments, which is Object. Such conclusion implies that, more precisely, the problem is not about explicitly defining a type argument, since write<Object>(countRef, 'banana') is also compile time valid. The actual point is the way the compiler fails to spot this particular incorrect construction.

The only way I see to solve this up is by not declaring both state and ref as parameters of the same method. I'd consider something like the following:

abstract class Scope {
  StateRefHandler<T> onStateRef<T>(StateRef<T> ref);
  ...
}

abstract class StateRefHandler<T> {
  void write<T>(T state, [Object action]);
  ...
}

There's a clear complexity increase on both library implementation and use, besides being a breaking change, but I believe it's worth it.

An alternative is discouraging calling Scope.write() without explicit type arguments. Perhaps a summary of this discussion could be placed somewhere at the documentation.

hcbpassos avatar Feb 09 '21 01:02 hcbpassos

Nicely spotted ! Thanks. Yes that's inconvenient, I need to find a syntax which is compile safe without adding a lot of complexity 🤔

letsar avatar Feb 12 '21 17:02 letsar

Hi, I will go with a solution highly inspired by you:

We will have this class responsible for mutating a state:

/// An objet which can mutate a state.
class StateMutator<T> {
  const StateMutator._(this.scope, this.ref);

  /// The scope where the actual state is stored.
  final Scope scope;

  /// The reference to the state.
  final StateRef<T> ref;

  /// {@macro binder.scope.write}
  void write(T state, [Object? action]) {
    scope.write(ref, state, action);
  }

  /// Updates the value of the state referenced by [ref] with a function which
  /// provides the current state.
  ///
  /// An optional [action] can be send to track which method did the update.
  void update(Updater<T> updater, [Object? action]) {
    scope.write(ref, updater(scope.read(ref, null)), action);
  }
}

And I will add a method in the Logic class to get an instance of a mutator.

mixin Logic {
  /// The scope where this logic is stored.
  @visibleForOverriding
  Scope get scope;

  StateMutator<T> withRef<T>(StateRef<T> ref){
    return StateMutator._(scope, ref);
  }
}

Then we would use it like this:

class MyLogic with Logic {
  void increment(){
    withRef(counterRef).update((x) => x++);
  }
}

I'm not sure of the name for the method used in the logic to get the mutator though. What do you think?

letsar avatar Mar 28 '21 09:03 letsar

Very interesting approach. My considerations:

  • There are methods that mutate the state but are not impacted by the described issue. Those are: clear(), undo(), redo(). They will remain inside Logic, right? If so, I wonder whether StateMutator is a good name. After all, the state may be mutated elsewhere as well.
  • This doesn't seem like an absolute solution, since it is still possible to use Scope directly:
final countRef = StateRef(0);

class CounterLogic with Logic {
  ...
  void increment() {
    withRef(countRef).write('banana'); // compiler: you're not fooling me you nasty boi.
    scope.write(countRef, 'banana'); // also compiler: yup, go ahead.
  }
}

About the name of the method to get the mutator, I think withRef() is fine. Initially, I though about onRef() because onRef().write() feels closer to "write on ref". However, now I think "write with ref" makes more sense because you're actually using the ref to write on state.

hcbpassos avatar Mar 28 '21 17:03 hcbpassos

Yes, you're right, Mutator is not a good name. Have you a better idea?

About the write method on scope, yes you're also right. If only we had an internal annotation, or something like that. I will think about something better, thanks!

letsar avatar Mar 28 '21 19:03 letsar

Ahh, hard to find a good name for that. I think I'll let you think of something better and, if after that the mutator still exists, then we think of a better name :)

hcbpassos avatar Mar 28 '21 21:03 hcbpassos