dart_sealed_unions icon indicating copy to clipboard operation
dart_sealed_unions copied to clipboard

Allow to filter which sealed unions you are listening

Open juliocbcotta opened this issue 6 years ago • 4 comments
trafficstars

I know this is particular to flutter, but maybe it makes sense to you too. Currently, we are able to call .join to listen all states, but sometimes we need a special handling for just one or two states. For instance the code below. I had to use multiple empty closures to listen to the error state and be able to show a SnackBar. Sadly, in flutter, we can't just return a SnackBar to the view tree. The same for Alerts and to navigation.

I see two options here. to create siblings methods to .join to allow filtering of the current state or to expose the current state type being emitted. I know this library is meant to allow a centralized state management, but it looks like that it is not always possible in Flutter.

body: BlocListener<LoginBloc, LoginState>(
            listener: (context, state) {
              state.join((_) {}, (_) {}, (_) {}, (error) {
                _scaffoldKey.currentState.showSnackBar(
                  SnackBar(
                    content: Text('Fuuuuuuuuuuuu $error'),
                  ),
                );
              });
            },
            child: BlocBuilder<LoginBloc, LoginState>(
              builder: (context, state) {
                return Stack(
                  children: <Widget>[
                    _buildLoginForm(),
                    state.join(
                        (initial) => Container(),
                        (loading) => Positioned.fill(
                            child: Center(child: CircularProgressIndicator())),
                        (logged) => Container(),
                        (error) => Container()),
                  ],
                );
              },
            ),
          )

Am I missing something? Is there any other way to handle this? Thanks.

cc: @felangel , maybe you have some input to this.

Edit: Simplified code.

juliocbcotta avatar Sep 17 '19 20:09 juliocbcotta

Hi, I understand that you might see this as a useful tool, but the approach would be an anti-pattern.

By implementing an only listen to these scenario to work around this mechanism would defeat the purpose of this library and leave your code open to error.

What Sealed Unions gives you is a Closed Inheritance, that means that you are only limited to all the inheritance types that are the defined in this file and none else which means that when the compiler goes to them it knows exactly that there has to be four different ones and that gives you some guarantees and that is good for development.

The purpose of using this type of library is so that you are assured that you have in-fact covered all possible cases.

nodinosaur avatar Sep 18 '19 14:09 nodinosaur

Well, then... Is this the wrong tool to my needs in the sample above? Because it does not look right having to place all those empty closures.

The real problem I see is that I don't have access to which Union I received without using join method. Being able to match all possible child classes is great, but should we be limited by that?

juliocbcotta avatar Sep 18 '19 16:09 juliocbcotta

It may be an anti-pattern to sealed classes, but there are still legitimate use cases for this. Another example might be an event bus listener that only listens for particular types of events. Having to provide a default implementation for every other kind of event just increases the already slightly annoying amount of boilerplate. As such, there's an argument for having a joinOrDefault method for when you're using sealed classes but don't need to exhaustively implement handling for each case everywhere you reference them.

andrewackerman avatar Nov 17 '19 15:11 andrewackerman

Did the developers of this package come to a conclusion on this? Based on the non-action, it seems like developers went the purist route, but I'm not certain because the issue isn't closed.

If we're open to a PR for this I would be happy to look into it - I have a union5 with a listener that looks like this:

BlocListener<MyBloc, MyState>(
   listener: (_, state) => state.join(
       (_) => null,
       (_) => null,
       (_) => doSomething(),
       (_) => null,
       (_) => null,
   )
)

another unfortunate side effect hold the boilerplate is that it's crushing my test coverage - that's 4 uncovered paths that don't do anything at all.

If the maintainers are open to a PR for something like joinOrDefault or otherwise, then let me know. Even in the inspiration language Rust, you can specify a default argument as opposed to needing to explicitly handle all states.

cc @nodinosaur - if you're open to a PR let me know!

antholeole avatar Aug 15 '21 20:08 antholeole