Skip to content
This repository has been archived by the owner on Dec 18, 2018. It is now read-only.

*CoreAsync vs *Async #2239

Closed
martore opened this issue May 9, 2018 · 7 comments
Closed

*CoreAsync vs *Async #2239

martore opened this issue May 9, 2018 · 7 comments
Milestone

Comments

@martore
Copy link

martore commented May 9, 2018

I've noticed 2.1 introduced new methods with core infix. Comparing SendCoreAsync and SendAsync, is there any specific difference between the two of them other than SendCoreAsync not being an extension (beside array of arguments as a parameter)? Which one should we use?

@analogrelay
Copy link
Contributor

The short answer is: The Core methods should be ignored unless you really know what you're doing.

For some more background:

We started with SendAsync, which takes an array of arguments to send:

public void SendAsync(string method, object[] args);

Clients.All.SendAsync("Method", new object[] { arg1, arg2, arg3 });

Obviously it's a pain to have to create an array every time. The easy fix for that would be to use params:

public void SendAsync(string method, params object[] args);

Clients.All.SendAsync("Method", arg1, arg2, arg3);

However, that falls apart when you actually want to send an array as a single argument

public void SendAsync(string method, params object[] args);

var arg1 = new object[] { a, b, c };

Clients.All.SendAsync("Method", arg1);

// C# 'params' expands this into the below

Clients.All.SendAsync("Method", a, b, c);

So instead of sending a single argument that is an array a, b, c, we've sent each of those as separate arguments. This was confusing users.

So we removed the params from it and instead we generate a whole bunch of extension methods that support multiple arguments:

public void SendAsync(string method, object[] args);
public void SendAsync(string method, object arg1) => SendAsync(method, new object[] { arg1 });
public void SendAsync(string method, object arg1, object arg2) => SendAsync(method, new object[] { arg1, arg2 });
// ... etc ...

But there's still ambiguity when you have code like this:

public void SendAsync(string method, object[] args);
public void SendAsync(string method, object arg1) => SendAsync(method, new object[] { arg1 });

var arg = new object[] { a, b, c }

Clients.All.SendAsync("method", arg);

Again, the overload that takes an object[] will be chosen (see this illustration on SharpLab).

So, we renamed the one that takes an array to SendCoreAsync:

public void SendCoreAsync(string method, object[] args);
public void SendAsync(string method, object arg1) => SendCoreAsync(method, new object[] { arg1 });

var arg = new object[] { a, b, c }

// No ambiguity here!
Clients.All.SendAsync("method", arg);

We will have XML docs that show up in IntelliSense in the RTM to help explain this.
We support up to 10 arguments this way (see https://github.com/aspnet/SignalR/blob/dev/src/Microsoft.AspNetCore.SignalR.Client.Core/HubConnectionExtensions.SendAsync.cs).

@analogrelay analogrelay added this to the Discussions milestone May 9, 2018
@martore
Copy link
Author

martore commented May 9, 2018

Thank you for explanation.

@Ro3A
Copy link

Ro3A commented Jul 25, 2018

@anurse I'm currently sending a single list of objects using SendAsync. That extension methods aren't supported by some mocking frameworks. I can't use SendCoreAsync either for the reasons above.

Are there near-term plans to make these extension methods instance methods or will I need to abstract these calls out to a wrapper class of some kind?

@analogrelay
Copy link
Contributor

The extension methods all just call SendCoreAsync though. You should be able to use them from your code but mock out SendCoreAsync.

@Ro3A
Copy link

Ro3A commented Jul 26, 2018

You're, right. At one point I tried that and it failed but later found it was due to something unrelated.

Thanks for the response.

@myrup
Copy link

myrup commented Aug 10, 2018

Explanation makes sense. As a sidenote, I was initially drawn to the "SendCore" because "Core" could refer to a .Net "Core" feature which is a common pattern especially in NuGet naming (e.g. "EntityFrameworkCore").

@analogrelay
Copy link
Contributor

@myrup This generally isn't the case in method names, but that's a good point to make.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants