mponkin / fading_edge_scrollview

Flutter library for displaying fading edges on scroll views
BSD 3-Clause "New" or "Revised" License
43 stars 30 forks source link

Why have multiple constructors when they all call the same internal constructor with the same arguments #9

Open moda20 opened 4 years ago

moda20 commented 4 years ago

I found out that you are creating constructors for no reason when you don't do anything special for each constructor : here check these constructors they have the same implementation but with different constructor arguments :

  /// Constructor for creating [FadingEdgeScrollView] with [PageView] as child
  /// child must have [PageView.controller] set
  factory FadingEdgeScrollView.fromPageView({
    Key key,
    @required PageView child,
    double gradientFractionOnStart = 0.1,
    double gradientFractionOnEnd = 0.1,
    bool shouldDisposeScrollController = false,
  }) {
    assert(child.controller != null, "Child must have controller set");

    return FadingEdgeScrollView._internal(
      key: key,
      child: child,
      scrollController: child.controller,
      scrollDirection: child.scrollDirection,
      reverse: child.reverse,
      gradientFractionOnStart: gradientFractionOnStart,
      gradientFractionOnEnd: gradientFractionOnEnd,
      shouldDisposeScrollController: shouldDisposeScrollController,
    );
  }

  /// Constructor for creating [FadingEdgeScrollView] with [AnimatedList] as child
  /// child must have [AnimatedList.controller] set
  factory FadingEdgeScrollView.fromAnimatedList({
    Key key,
    @required AnimatedList child,
    double gradientFractionOnStart = 0.1,
    double gradientFractionOnEnd = 0.1,
    bool shouldDisposeScrollController = false,
  }) {
    assert(child.controller != null, "Child must have controller set");

    return FadingEdgeScrollView._internal(
      key: key,
      child: child,
      scrollController: child.controller,
      scrollDirection: child.scrollDirection,
      reverse: child.reverse,
      gradientFractionOnStart: gradientFractionOnStart,
      gradientFractionOnEnd: gradientFractionOnEnd,
      shouldDisposeScrollController: shouldDisposeScrollController,
    );
  }

why is that ?

mponkin commented 4 years ago

All constructors are extracting required parameters (scrollController, scrollDirection, reverse) from child widgets. Unfortunately scrollable widgets don't share same interface for this purpose even if their methods are looking similar.

timukasr commented 7 months ago

You could allow any Widget and extract based on type:

  factory FadingEdgeScrollView({
    Key? key,
    required Widget child,
    double gradientFractionOnStart = 0.1,
    double gradientFractionOnEnd = 0.1,
  }) {
    final (controller, scrollDirection, reverse) = _extractFromChild(child);

    if (controller == null) {
      throw Exception("Child must have controller set");
    }

    return FadingEdgeScrollView._internal(
      key: key,
      scrollController: controller,
      scrollDirection: scrollDirection,
      reverse: reverse,
      gradientFractionOnStart: gradientFractionOnStart,
      gradientFractionOnEnd: gradientFractionOnEnd,
      child: child,
    );
  }

  static (ScrollController? controller, Axis scrollDirection, bool reverse) _extractFromChild(Widget widget) {
    switch (widget) {
      case (ScrollView widget):
        return (widget.controller, widget.scrollDirection, widget.reverse);
      case (ReorderableListView widget):
        return (widget.scrollController, widget.scrollDirection, widget.reverse);
      case (SingleChildScrollView widget):
        return (widget.controller, widget.scrollDirection, widget.reverse);
      case (PageView widget):
        return (widget.controller, widget.scrollDirection, widget.reverse);
      case (AnimatedList widget):
        return (widget.controller, widget.scrollDirection, widget.reverse);
      case (ListWheelScrollView widget):
        return (widget.controller, Axis.vertical, false);
      default:
        throw Exception(
          "Child must be ScrollView, ReorderableListView, SingleChildScrollView, PageView, AnimatedList or ListWheelScrollView",
        );
    }
  }