GIfatahTH / states_rebuilder

a simple yet powerful state management technique for Flutter
494 stars 56 forks source link

Multiple Mixins, no nesting? #19

Closed burnsauce closed 5 years ago

burnsauce commented 5 years ago

I am reviewing various state management paradigms/libraries available for flutter, and I have trouble refactoring my application according to your mixin provider scheme because my existent widget class implements multiple mixins.

The StateWithMixinBuilder's mixinWith parameter is not a List, which seems strange, as mixin flexibility stems from its multiplicity.

I hesitate to continue the refactor in this case. Am I missing a more obvious solution? The two mixins I use are TickerProviderStateMixin, WidgetsBindingObserver.

GIfatahTH commented 5 years ago

mixinWith parameter is en enum, so that you can use only on mixin at a time. To use more than one mixin, nest stateWithMixinBuilders.

Look at this snipped of code from the example : https://github.com/GIfatahTH/states_rebuilder/blob/master/example/lib/state_with_mixin_builder.dart/pages/9_rebuildStates_performance.dart

I used WidgetsBindingObserver and singleTickerProviderStateMixin;

and in this example: https://github.com/GIfatahTH/states_rebuilder/blob/master/example/lib/state_with_mixin_builder.dart/pages/2_rebuild_one.dart

I used automaticKeepAliveClientMixinand singleTickerProviderStateMixin;

In this code, WidgetsBindingObserver and singleTickerProviderStateMixin are used:

class CounterGrid extends StatelessWidget {
  final bloc = Injector.get<CounterBlocPerf>();
  @override
  Widget build(BuildContext context) {
    return Padding(
      padding: EdgeInsets.all(10),
      child: StateWithMixinBuilder<WidgetsBindingObserver>(
        mixinWith: MixinWith.widgetsBindingObserver,
        initState: (_, __, observer) =>
            WidgetsBinding.instance.addObserver(observer),
        dispose: (_, __, observer) =>
            WidgetsBinding.instance.removeObserver(observer),
        didChangeAppLifecycleState: (context, tagID, state) {
          bloc.lifecycleState(context, tagID, state);
        },
        builder: (_, __) => Column(
              children: <Widget>[
                StateBuilder(
                    viewModels: [bloc],
                    tag: CounterTag.time,
                    builder: (_, __) => Column(
                          children: <Widget>[
                            Text(
                                "Time taken to execute rebuildStates() ${bloc.time}"),
                            Text(
                                "This page is mixin with widgetsBindingObserver. The state is: ${bloc.state}"),
                          ],
                        )),
                Expanded(
                  child: StateWithMixinBuilder(
                    mixinWith: MixinWith.singleTickerProviderStateMixin,
                    initState: (_, __, ticker) => bloc.initAnimation(ticker),
                    dispose: (_, __, ___) => bloc.dispose(),
                    builder: (_, __) => GridView.count(
                          crossAxisCount: 3,
                          children: <Widget>[
                            for (var i = 0; i < 12; i++)
                              StateBuilder(
                                tag: i % 2,
                                viewModels: [bloc],
                                builder: (_, tagID) => Transform.rotate(
                                      angle: bloc.animation.value,
                                      child: GridItem(
                                        count: bloc.counter,
                                        onTap: () =>
                                            bloc.triggerAnimation(i % 2),
                                      ),
                                    ),
                              ),
                          ],
                        ),
                  ),
                ),
              ],
            ),
      ),
    );
  }
}
burnsauce commented 5 years ago

I understand that nesting could solve the problem, but I put it to you that mixins are designed to be used together.

That is: how you have structured this aspect of the library is not idiomatic to dart. It introduces combinatorial complexity to a feature that typically helps to eliminate that very thing!

None of the other state management libraries I have encountered so far have asked me to break up my widget in this manner.

Additionally, I will note that it seems off to store an animation controller in the business logic object. It's hardly business logic that something is fading in. Correct me if I'm wrong.

GIfatahTH commented 5 years ago

Let me be clear. The idea behind `StateWithMixinBuilder is to limit the use of statefulWidget to the minimum possible, because it is a little boilerplately. For pure state management you can use injector and StateBuilder.

This is an example of mixing with TickerProviderStateMixin and WidgetsBindingObserver

class APP extends StatefulWidget {
  @override
  _APPState createState() => _APPState();
}

class _APPState extends State<APP> with TickerProviderStateMixin, WidgetsBindingObserver {

@override
  void initState() {
    super.initState();
    // Your code to set TickerProvider and WidgetBindingObserver

  }

  @override
  Widget build(BuildContext context) {
    return StateBuilder(
      viewModels: [],
      builder: (_,__)=>Container(

      ),

    );
  }
}

the code above is equivalent to the code bellow where I split TickerProviderStateMixin and WidgetsBindingObserver to its own class. I know taht mixins are meant to be used together. But what is the problem (in term of performance) with idea of nesting mixin especially if mixin are independent?

// first class
class AnimationApp extends StatefulWidget {
  @override
  _AnimationAppState createState() => _AnimationAppState();
}

class _AnimationAppState extends State<AnimationApp>
    with TickerProviderStateMixin {
  // Set animation controller

  @override
  void initState() {
    super.initState();
  }

  @override
  Widget build(BuildContext context) {
    return StateBuilder(
      viewModels: [model],
      builder: (_, __) => Container(),
    );
  }
}

//second class
class MyApp extends StatefulWidget {
  @override
  _MyAppState createState() => _MyAppState();
}

class _MyAppState extends State<MyApp> with WidgetsBindingObserver {
  // Set WidgetsBindingObserver

  @override
  void initState() {
    super.initState();
  }

  @override
  Widget build(BuildContext context) {
    return AnimationApp();
  }
}

this code is exactly equivalent to :

class MyApp extends StatelessWidget {
  @override
  Widget build(BuildContext context) {
    return StateWithMixinBuilder<WidgetsBindingObserver>(
      mixinWith: MixinWith.widgetsBindingObserver,
      initState: (context, _, observer) {

      },
      builder: (_, __) => StateWithMixinBuilder<TickerProviderStateMixin>(
        mixinWith: MixinWith.tickerProviderStateMixin,
        initState: (context, _, ticker) {

        },
        viewModels: [model],
        builder: (_, __) => Container(),
      ),
    );
  }
}

Finally, the idea to move animation controller in viewModel class is simply for the sake to make views sole responsibility to produce the UI and free them of the mess of setting controllers.

The bottom line is that "StateWithMixinBuilder" is used for only for convenience, You can stick to statefulWidget and instantiate controllers inside it.

You said that you haven seen this pattern in any other state management libraries. This means that the idea is novel and as you know many new ideas look to be silly ideas but this does not mean that they are wrong.

burnsauce commented 5 years ago

I understand. I suppose things just struck me as off. I can see now how this architecture echoes flutter's pattern of composition and provides clear benefit. Thank you for your time explaining your rationale.