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

Add first class support for method forwarding #3748

Open
natebosch opened this issue Nov 29, 2017 · 9 comments
Open

Add first class support for method forwarding #3748

natebosch opened this issue Nov 29, 2017 · 9 comments
Labels
feature Proposed language feature that solves one or more problems

Comments

@natebosch
Copy link
Member

natebosch commented Nov 29, 2017

It's a common pattern to wrap an interface with a Delegating class which forwards all methods to a instance of the same interface and allows selectively overriding methods.

For example see Delegating* classes in package:collection and package:async

It would be cool if this was automatic rather than needing to manually write these classes.

Here's a straw man proposal:

abstract class SomeInterface {
  String someMethod();
  int someOtherMethod();
}

class Foo implements SomeInterface {
  final delegate SomeInterface _delegate;
  Foo(this._delegate);

  @override
  String someMethod() => 'Foo implementation';

  // Implicitly:
  // int someOtherMethod() => _delegate.someOtherMethod();
}

If:

  • a field is marked delegate
  • The delegate field is statically determined to be of type Foo
  • The class implements Foo

Then:

  • Any methods in Foo which are not directly implemented in the class are automatically implemented by forwarding to the field marked delegate.

I think this also could work with multiple delegates and it would be a static error to have a method with identical signatures in two interfaces with ambiguous forwarding. This is similar to a static error when implementing two interfaces with the same method and conflicting signatures.

class Foo implements First, Second {
  final delegate First _first;
  final delegate Second _second;
  Foo(this._first, this._second);
}
@matanlurey
Copy link
Contributor

matanlurey commented Nov 29, 2017

It might be simpler to encapsulate at a higher level:

class Foo implements First, Second {
  delegate factory Foo(First first, Second second);
}

It would be simple enough to prototype this, for example, using source_gen:

part 'foo.g.dart';

abstract class Foo implements First, Second {
  @delegate factory Foo(First first, Second second) = _$Foo$Delegate;
  Foo._();
}

@lrhn
Copy link
Member

lrhn commented Nov 30, 2017

It's an interesting idea that I've been thinking about for a while.

There is a difference between forwarding and delegation. In the latter, the this object during the call will be the original object, not the one the call is forwarded to (like if it was an inherited superclass method, not a separate object). I don't think it's likely that we'll get proper delegation in Dart, it's too big a change, and it would prevent some optimizations that rely on knowing the type of this.

Forwarding is easier, it's just implicit forwading methods that you would have to write yourself otherwise. (and if we had forwarding syntax for methods, like for constructors, it would be even easier, then it really would just be an implciit foo(arg1, arg2) = _theFoo.foo;.
The biggest challenge is finding a syntax for linking the interface with the object, and handling conflicts.

class D {
  int foo() => 42;
}
class C implements D {
  delegate D =>_theD;
  D get _theD => ...
  ...
  // implicit:  int foo() => _theD.foo();
}

or

class C implements D as _theD {
  get D _theD => ...;
  ...
  // implicit:  int foo() => _theD.foo();
}

or something else.

Linking it up during construction is restrictive - you can't change the forwarded-to object while running (well, you can, you just have to work harder for it, so the restrictions are probably not buying much).

You need to handle conflicts. If you have two delegates to related interfaces (anything with common non-trivial supertype), you have two figure out where the conflicted methods go. Most likely, it would just be an error, and you'll have to handle those methods manually (If you delegate both List and Queue, the Iterable methods won't know where to go).

Maybe if one delegated type is a subtype of another, that one always win., and if you have multiple, one must be a subtype of all the rest. Then you can do:

class C implements Set<int>, List<int> {
  delegate Set<int> => theSet;
  delegate List<int> => theList;
  delegate Iterable<int> => _theSet;  // Solve conflict between Set and List.
  Set<int> get _theSet => ...;
  List<int> get _theList => ...;
}

As forwarding, it's pure syntactic sugar, but it's adaptive syntactic sugar. If the interface gets more members, you don't have to go back and add the extra methods to your forwarding wrappers. I've written enough such forwarding classes to consider that valuable.

@natebosch
Copy link
Member Author

There is a difference between forwarding and delegation. In the latter, the this object during the call will be the original object, not the one the call is forwarded to (like if it was an inherited superclass method, not a separate object).

Thanks for the clarification! By these terms forwarding is what I'd hope for and I'd explicitly not want delegation.

@a14n
Copy link

a14n commented Nov 30, 2017

Some times ago I wrote zengen that contains a generator to do delegation/forwarding. The annotation had an optional parameter to be able to exclude some methods and provide a way to manage conflicts between delegates.

@FMorschel
Copy link

FMorschel commented May 2, 2024

Since this came up recently and the last comment here is from more than 6 years ago, any status update on going towards this or similar? Anything in the language has changed so this is easier or harder to accomplish.

Thank you in advance for taking the time to answer.

@Levi-Lesches
Copy link

Levi-Lesches commented May 2, 2024

Going off the first example:

abstract class SomeInterface {
  String someMethod();
  int someOtherMethod();
}

class Foo implements SomeInterface {
  final delegate SomeInterface _delegate;
  Foo(this._delegate);

  @override
  String someMethod() => 'Foo implementation';

  // Implicitly:
  // int someOtherMethod() => _delegate.someOtherMethod();
}

You can use extension types to achieve the same thing:

extension type Foo (SomeInterface delegate) implements SomeInterface {
  @override
  String someMethod() => 'Foo implementation';
  
  // Implicitly:
  // int someOtherMethod() => delegate.someOtherMethod();
}

I would say this is pretty similar to the requested feature. @natebosch do you agree or did I miss something?

@FMorschel
Copy link

Yes, this is similar but it only works to implement the extended type. It does not work if you want to have multiple implementations/forwarding. Like the example above by Lasse:

class C implements Set<int>, List<int> {
  delegate Set<int> => theSet;
  delegate List<int> => theList;
  delegate Iterable<int> => _theSet;  // Solve conflict between Set and List.
  Set<int> get _theSet => ...;
  List<int> get _theList => ...;
}

@natebosch
Copy link
Member Author

I think extension types probably help in many cases. I don't think they solve the problem fully because they are only a static feature - I cannot pass my Foo as if it were a SomeInterface to other code if Foo is an extension type.

@FMorschel
Copy link

FMorschel commented May 6, 2024

I'm not sure this is the right place for me to inform you of this, but in #3747 I was talking about creating a DelegatingFocusNode class. I've fully forwarded all 48 methods/getters and setters to an existing FocusNode with it.

Here is the code
class DelegatingFocusNode implements FocusNode {
  DelegatingFocusNode({
    String? debugLabel,
    // ignore: deprecated_member_use, api needed
    FocusOnKeyCallback? onKey,
    FocusOnKeyEventCallback? onKeyEvent,
    bool skipTraversal = false,
    bool canRequestFocus = true,
    bool descendantsAreFocusable = true,
    bool descendantsAreTraversable = true,
  }) : _base = FocusNode(
          canRequestFocus: canRequestFocus,
          debugLabel: debugLabel,
          descendantsAreFocusable: descendantsAreFocusable,
          descendantsAreTraversable: descendantsAreTraversable,
          // ignore: deprecated_member_use, api needed
          onKey: onKey,
          onKeyEvent: onKeyEvent,
          skipTraversal: skipTraversal,
        );

  DelegatingFocusNode.fromClass(this._base);

  final FocusNode _base;

  @override
  bool get canRequestFocus => _base.canRequestFocus;

  @override
  set canRequestFocus(bool value) => _base.canRequestFocus = value;

  @override
  String? get debugLabel => _base.debugLabel;

  @override
  set debugLabel(String? value) => _base.debugLabel = value;

  @override
  bool get descendantsAreFocusable => _base.descendantsAreFocusable;

  @override
  set descendantsAreFocusable(bool value) =>
      _base.descendantsAreFocusable = value;

  @override
  bool get descendantsAreTraversable => _base.descendantsAreTraversable;

  @override
  set descendantsAreTraversable(bool value) =>
      _base.descendantsAreTraversable = value;

  @override
  // ignore: deprecated_member_use, api needed
  FocusOnKeyCallback? get onKey => _base.onKey;

  @override
  // ignore: deprecated_member_use, api needed
  set onKey(FocusOnKeyCallback? value) => _base.onKey = value;

  @override
  FocusOnKeyEventCallback? get onKeyEvent => _base.onKeyEvent;

  @override
  set onKeyEvent(FocusOnKeyEventCallback? value) => _base.onKeyEvent = value;

  @override
  bool get skipTraversal => _base.skipTraversal;

  @override
  set skipTraversal(bool value) => _base.skipTraversal = value;

  @override
  void addListener(VoidCallback listener) => _base.addListener(listener);

  @override
  Iterable<FocusNode> get ancestors => _base.ancestors;

  @override
  FocusAttachment attach(
    BuildContext? context, {
    FocusOnKeyEventCallback? onKeyEvent,
    // ignore: deprecated_member_use, api needed
    FocusOnKeyCallback? onKey,
  }) =>
      // ignore: deprecated_member_use, api needed
      _base.attach(context, onKeyEvent: onKeyEvent, onKey: onKey);

  @override
  Iterable<FocusNode> get children => _base.children;

  @override
  bool consumeKeyboardToken() => _base.consumeKeyboardToken();

  @override
  BuildContext? get context => _base.context;

  @override
  List<DiagnosticsNode> debugDescribeChildren() =>
      _base.debugDescribeChildren();

  @override
  void debugFillProperties(DiagnosticPropertiesBuilder properties) =>
      _base.debugFillProperties(properties);

  @override
  Iterable<FocusNode> get descendants => _base.descendants;

  @override
  void dispose() => _base.dispose();

  @override
  FocusScopeNode? get enclosingScope => _base.enclosingScope;

  @override
  bool focusInDirection(TraversalDirection direction) =>
      _base.focusInDirection(direction);

  @override
  bool get hasFocus => _base.hasFocus;

  @override
  bool get hasListeners => _base.hasListeners;

  @override
  bool get hasPrimaryFocus => _base.hasPrimaryFocus;

  @override
  FocusHighlightMode get highlightMode => _base.highlightMode;

  @override
  FocusScopeNode? get nearestScope => _base.nearestScope;

  @override
  bool nextFocus() => _base.nextFocus();

  @override
  void notifyListeners() => _base.notifyListeners();

  @override
  Offset get offset => _base.offset;

  @override
  FocusNode? get parent => _base.parent;

  @override
  bool previousFocus() => _base.previousFocus();

  @override
  Rect get rect => _base.rect;

  @override
  void removeListener(VoidCallback listener) => _base.removeListener(listener);

  @override
  void requestFocus([FocusNode? node]) => _base.requestFocus(node);

  @override
  Size get size => _base.size;

  @override
  DiagnosticsNode toDiagnosticsNode({
    String? name,
    DiagnosticsTreeStyle? style,
  }) =>
      _base.toDiagnosticsNode(name: name, style: style);

  @override
  String toStringDeep({
    String prefixLineOne = '',
    String? prefixOtherLines,
    DiagnosticLevel minLevel = DiagnosticLevel.debug,
  }) =>
      _base.toStringDeep(
        prefixLineOne: prefixLineOne,
        prefixOtherLines: prefixOtherLines,
        minLevel: minLevel,
      );

  @override
  String toStringShallow({
    String joiner = ', ',
    DiagnosticLevel minLevel = DiagnosticLevel.debug,
  }) =>
      _base.toStringShallow(minLevel: minLevel, joiner: joiner);

  @override
  String toStringShort() => _base.toStringShort();

  @override
  String toString({DiagnosticLevel minLevel = DiagnosticLevel.info}) =>
      _base.toString(minLevel: minLevel);

  @override
  Iterable<FocusNode> get traversalChildren => _base.traversalChildren;

  @override
  Iterable<FocusNode> get traversalDescendants => _base.traversalDescendants;

  @override
  void unfocus({UnfocusDisposition disposition = UnfocusDisposition.scope}) =>
      _base.unfocus(disposition: disposition);
}

And even with all this work I even got an error when using this DelegatingFocusNode inside a TextFormField with autofocus set to true.

Error:

════════ Exception caught by scheduler library ═════════════════════════════════
The following NoSuchMethodError was thrown during a scheduler callback:
Class 'FocusNodeTextController' has no instance getter '_parent'.
Receiver: Instance of 'FocusNodeTextController'
Tried calling: _parent

When the exception was thrown, this was the stack:
#0      DelegatingFocusNode._parent (package:custom_fields/src/focus_node_text_controller.dart:140:7)
focus_node_text_controller.dart:140
#1      FocusScopeNode.autofocus (package:flutter/src/widgets/focus_manager.dart:1368:14)
focus_manager.dart:1368
#2      EditableTextState.didChangeDependencies.<anonymous closure> (package:flutter/src/widgets/editable_text.dart:2883:34)
editable_text.dart:2883
#3      SchedulerBinding._invokeFrameCallback (package:flutter/src/scheduler/binding.dart:1386:15)
binding.dart:1386
#4      SchedulerBinding.handleDrawFrame (package:flutter/src/scheduler/binding.dart:1322:11)
binding.dart:1322
#5      SchedulerBinding._handleDrawFrame (package:flutter/src/scheduler/binding.dart:1169:5)
binding.dart:1169

So I believe as the language is now, some things can't be fully done by delegating manually to a base class and that is something to be considered IMO.

Edit:

Previously to creating this class, I was extending the FocusNode and of course got no issues since even private methods were being implicitly created.

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

6 participants