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

fix: fixed chainFirst returning Left result of chained method #47

Merged
merged 2 commits into from
Aug 19, 2022

Conversation

DevNico
Copy link
Contributor

@DevNico DevNico commented Aug 15, 2022

Closes #46

@DevNico
Copy link
Contributor Author

DevNico commented Aug 15, 2022

You should probably add a fail call everywhere you do a match to expect something in either left or right cases since this way tests that should fail, pass.

@SandroMaglione
Copy link
Owner

SandroMaglione commented Aug 19, 2022

Hi @DevNico

Good catch! You are right, chainFirst is incorrect when chaining fails.

Would you mind fixing chainFirst also in the other types? Namely:

  • Either
  • TaskEither
  • Monad
  • State
  • StateAsync
  • Reader
  • IOEither

@DevNico
Copy link
Contributor Author

DevNico commented Aug 19, 2022

I fixed the IOEither case. I have no experience using those other types and don't see them being broken just by looking at the code. If there is something wrong with the code there I suggest you fix it.

@SandroMaglione
Copy link
Owner

Great, thanks again for your contribution and suggestion 👍

@SandroMaglione SandroMaglione merged commit 4369daf into SandroMaglione:main Aug 19, 2022
@rd-martin-jaeger
Copy link

rd-martin-jaeger commented May 8, 2023

Hi @SandroMaglione @DevNico,

i have a general question about the function, regardless of how it is currently described in the documentation.
Shouldn't chainFirst() actually be able to return any failures instead of ignoring them?

Thats at least what i would expect when i start to chain eithers or did i miss the point, where it totally makes sense to ignore them within you chained functions?

Consequently, the previous solution would actually have been the right one for me.

flatMap((b) => chain(b).map((c) => b));
group('chainFirst', () {
    test('Right', () async {
     final task = TaskEither<String, int>.of(10);

     final chain = task.chainFirst((b) => TaskEither.right(100));

     final result = await chain.run();

     result.match(
       (l) => fail('should be right'),
       (r) => expect(r, 10),
     );
   });

   group('Left', () async {
     final task = TaskEither<String, int>.of(10);

     final chain = task.chainFirst((b) => TaskEither.left('none'));

     final result = await chain.run();

     result.match(
       (l) => expect(l, 'none'),
       (r) => fail('should be left'),
     );
   });
 });

@SandroMaglione
Copy link
Owner

Hi @rd-martin-jaeger

If you need to return any failure you should use flatMap. chainFirst instead is used precisely for ignoring: you chain some computations without collecting its result.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

chainFirst only works for successful cases
3 participants