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

Implement Couchbase.Extensions.MultiOp #92

Merged
merged 3 commits into from
Mar 17, 2021
Merged

Implement Couchbase.Extensions.MultiOp #92

merged 3 commits into from
Mar 17, 2021

Conversation

brantburnett
Copy link
Collaborator

Motivation

Provide a standardized, yet flexible, approach for running multiple
operations in parallel which is simpler and more performant than other
options using Tasks.

Modifications

Create the Couchbase.Extensions.MultiOp package with a variety of tests.

Motivation
----------
Provide a standardized, yet flexible, approach for running multiple
operations in parallel which is simpler and more performant than other
options using Tasks.

Modifications
-------------
Create the Couchbase.Extensions.MultiOp package with a variety of tests.
/// <remarks>
/// Operations are not executed until the observable is subscribed.
/// </remarks>
public static IObservable<MultiOpResult<IGetResult>> Get(this ICouchbaseCollection collection,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Personally, I like the Get without the Async postfix, however, all of the feedback we got when sdk3 was in early pre-release was that people didn't like it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I was one of the people who insisted on Async because it gives a clear indication of the method's design. However, I think in this case it actually causes more confusion. It gives the impression that this method returns Task<T>, which it doesn't. Worse, it will encourage users to await the return value. If the IObservable is awaited, then it will wait for all operations to complete but then only return the value of the last operation, dropping all others. I can see this being very confusing.

Perhaps we should consider other method names instead? Like MultiGet instead of Get? Do you have any other ideas?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gotcha, I recall we discussed this before (were not awaiting a Task, its an observable). Yeah, I think MultiGet over Get is better, we handle the "Async" potential confusion in docs.

@jeffrymorris jeffrymorris merged commit 06b3543 into master Mar 17, 2021
@brantburnett brantburnett deleted the multiop branch March 20, 2021 21:59
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.

2 participants