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

Rename toList/toArray to toListSynchronously/toArraySynchronously #98

Open
blumu opened this issue Dec 29, 2018 · 2 comments
Open

Rename toList/toArray to toListSynchronously/toArraySynchronously #98

blumu opened this issue Dec 29, 2018 · 2 comments

Comments

@blumu
Copy link

blumu commented Dec 29, 2018

Calling Async.RunSynchronously in libraries is error-prone and can cause dead-locks. (e.g. when running on the main interactive thread.) I would just remove the two functions toList and toArray altogether to prevent library users from hitting hard-to-debug issues.

let toList (source:AsyncSeq<'T>) = toListAsync source |> Async.RunSynchronously

let toArray (source:AsyncSeq<'T>) = toArrayAsync source |> Async.RunSynchronously

@dsyme
Copy link
Contributor

dsyme commented Jan 8, 2019

It's true, though I certainly remember using these when playing around with AsyncSeq, testing and so on. Perhaps renaming to toListSynchronously and and toArraySynchronously may be right for now?

@eulerfx
Copy link
Contributor

eulerfx commented Jan 9, 2019

I'd second the rename. I agree that use of Async.RunSynchronously is troublesome and that these operations conceal it (particularly from a code quality scanner).

blumu pushed a commit to blumu/FSharp.Control.AsyncSeq that referenced this issue Jan 13, 2019
@dsyme dsyme changed the title toList and toArray should not call Async.RunSynchronously Rename toList/toArray to toListSynchronously/toArraySynchronously Jul 22, 2020
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

No branches or pull requests

3 participants