Skip to content
This repository has been archived by the owner on Oct 17, 2024. It is now read-only.

Expand docs for isCompleted #104

Merged
merged 2 commits into from
Mar 16, 2020
Merged

Expand docs for isCompleted #104

merged 2 commits into from
Mar 16, 2020

Conversation

natebosch
Copy link
Contributor

Closes dart-lang/core#327

This field is confusing because it documents whether the backing
completer has had complete called which doesn't necessarily correspond
to when the value is available if it was completed with a Future.

Closes #102

This field is confusing because it documents whether the backing
completer has had `complete` called which doesn't necessarily correspond
to when the value is available if it was completed with a `Future`.
@natebosch natebosch requested a review from lrhn March 13, 2020 17:10
lib/src/cancelable_operation.dart Outdated Show resolved Hide resolved
/// Whether the [CancelableCompleter] backing this operation has been
/// completed.
///
/// This does not indicate that the completer backing [value] has been
Copy link
Contributor

Choose a reason for hiding this comment

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

/// This value being true does not imply that the [value] future has completed, 
/// but merely that it is no longer possible to [cancel] the operation.

Maybe this value is not that useful. I would personally prefer to not have isCompleted at all, or have named it canCancel. Probably not worth the breaking change, though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This raises a question - if it is not possible to cancel a CancelableOperation.fromFuture, why does that constructor exists? What do we get out of wrapping if we can't cancel?

Copy link
Contributor

Choose a reason for hiding this comment

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

You get to pass a Future to something expecting a CancelableOperation. It's an adapter, partial because the adeptee doesn't support the full API it's being adapted for.

I would probably have allowed you to cancel until the future result was available, whether the underlying completer is completed or not, but that would require more complicated plumbing - not just completing the underlying operation/future completers with the future.

@natebosch natebosch merged commit c7eb685 into master Mar 16, 2020
@natebosch natebosch deleted the cancellable-docs branch March 16, 2020 20:30
mosuem pushed a commit to dart-lang/core that referenced this pull request Oct 14, 2024
Closes dart-lang/async#102

This field is confusing because it documents whether the backing
completer has had `complete` called which doesn't necessarily correspond
to when the value is available if it was completed with a `Future`.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Development

Successfully merging this pull request may close these issues.

CancelableOperation.isCompleted is True Before Future Completes
3 participants