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

Expose the internal task of the send operations as a return value #126

Merged
merged 3 commits into from Feb 23, 2015

Conversation

galbarm
Copy link
Collaborator

@galbarm galbarm commented Feb 15, 2015

Exposing the Task object gives the caller to the the Send method, an access to asynchronous send operation.

An example benefit is that if the caller needs to guarantee that an order is being kept between consecutive
send operations, he can wait for the task to complete and only then call Send again.

@galbarm galbarm mentioned this pull request Feb 15, 2015
@statianzo
Copy link
Owner

Why was OnPing changed? Other than that, it looks good.

@galbarm
Copy link
Collaborator Author

galbarm commented Feb 16, 2015

I originally changed it to match the signature of SendPong since it was assigned with it in the constructor:
OnPing = SendPong;

However, it seems like it was unnecessary since I could just change the assignment to:
OnPing = x => SendPong(x);

So I added another commit with this change.

{
if (Handler == null)
throw new InvalidOperationException("Cannot send before handshake");

if (!IsAvailable) {
FleckLog.Warn("Data sent while closing or after close. Ignoring.");
return;
return null;
Copy link
Owner

Choose a reason for hiding this comment

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

This should return a faulted task so consumers of Send don't need to do an "if null" check and can choose to handle it through ContinuationOptions.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

How should I do that?
The Task's Status property is read-only, so it's impossible to create a task only for the sake of returning it with a Faulted status.
Also, a task was not yet created at this point, so it's not necessary to handle it in the way you suggested, other than for keeping the consumer code clean.

The only way I can think to accomplish it is to create a task, throw exception in it's body, start it, and return it.
Is this what you had in mind?

Copy link
Owner

Choose a reason for hiding this comment

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

As ugly as that is, creating a task with a raised exception in it would be the bast way to handle the scenario. Throwing something like Fleck.ConnectionNotAvailableException is more descriptive and self-documenting than a null reference.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

statianzo added a commit that referenced this pull request Feb 23, 2015
Expose the internal task of the send operations as a return value
@statianzo statianzo merged commit 788daae into statianzo:master Feb 23, 2015
@statianzo
Copy link
Owner

Great! Thanks for your work on this.

statianzo added a commit that referenced this pull request Feb 24, 2015
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

2 participants