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 Promise#filter #302

Merged
merged 1 commit into from May 22, 2012
Merged

Fix Promise#filter #302

merged 1 commit into from May 22, 2012

Conversation

julienrf
Copy link
Contributor

#459

  • Fix Promise#filter so it never leads to an unredeemable promise ;
  • Follow the Scala standard Future#filter behavior when the predicate fails: throw a NoSuchElementException ;
  • Remove STMPromise#collect which was wrong and anyway not expose at the Promise interface level.

@ghost ghost assigned sadache May 16, 2012
@pk11
Copy link
Contributor

pk11 commented May 16, 2012

Hi @sadache, it looks OK to me but could you please check this?

@sadache
Copy link
Contributor

sadache commented May 16, 2012

Yes, Ok for me too. But the pull request needs to be cleaned from the two other unrelated commits. This means we integrate only 9d69b11.
Thanks Julien for this fix.

@sadache
Copy link
Contributor

sadache commented May 21, 2012

Actually not so sure any more.
Maybe leaving filter as is and making collect public is better.
The idea is that you can combine two or more filtered promises using "or".

@julienrf
Copy link
Contributor Author

So you want to leave the filter implementation as is (silently blocking? See the related ticket on the tracker)? This implementation is different than the one in scala 2.10. The same applies for collect.

On 21 mai 2012, at 20:02, Sadek Drobi reply@reply.github.com wrote:

Actually not so sure any more.
Maybe leaving filter as is and making collect public is better.
The idea is that you can combine two or more filtered promises using "or".


Reply to this email directly or view it on GitHub:
#302 (comment)

@sadache
Copy link
Contributor

sadache commented May 21, 2012

Nothing is blocking, but it never redeems.
I want to find a way to keep the very interesting and practical functionality to "or" different potentially redeeming promises.

@sadache
Copy link
Contributor

sadache commented May 21, 2012

clean the pull request so that I can pull it before I change my mind

@julienrf
Copy link
Contributor Author

Ok, I understand your points. There is a Future#either which seems to perform the Promise#or of Play, unless I’m wrong?

@julienrf
Copy link
Contributor Author

@sadache
Copy link
Contributor

sadache commented May 22, 2012

The Future#either would return NoSuchElement if the first redeemed future is out of the filter (whereas the second could be satisfactory)

sadache added a commit that referenced this pull request May 22, 2012
@sadache sadache merged commit c45771d into playframework:master May 22, 2012
@julienrf
Copy link
Contributor Author

Oh yes, you’re right… We could write a orSuccess which would perform what you said?

@sadache
Copy link
Contributor

sadache commented May 22, 2012

Won't work, I want to use error in worst case scenario (both promises return an error), ignoring NoSuchElement is wrong because it could happen from something else.

@julienrf julienrf deleted the fix/459 branch January 14, 2013 09:15
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.

None yet

3 participants