3
\$\begingroup\$

I am using SignalR to send notifications to clients. I use IClientProxy which is a proxy for invoking hub methods.

It has 10 overload methods for SendAsync that takes parameters one by one as you see below, however, to handle this situation I have written a spagetti style method which is really weird to me.

Could you please give some suggestion?

private async Task InvokeNotification(string userName, string methodName, params object[] parameters)
{
    if (connections.ContainsKey(userName))
    {
        var proxy = Clients.Clients(connections[userName]);
        switch (parameters.Length)
        {
            case 0:
                await proxy.SendAsync(methodName);
                break;

            case 1:
                await proxy.SendAsync(methodName, parameters[0]);
                break;

            case 2:
                await proxy.SendAsync(methodName, parameters[0], parameters[1]);
                break;

            default:
                throw new HubException($"No method was defined with {parameters.Length} parameters in Hub");
        }
    }
}

The following code block is the definition of SendAsync which is coming from SignalR

public static class ClientProxyExtensions
{
    public static Task SendAsync(this IClientProxy clientProxy, string method, object arg1, object arg2, object arg3, object arg4, object arg5, object arg6, object arg7, object arg8, object arg9, object arg10, CancellationToken cancellationToken = default);
    public static Task SendAsync(this IClientProxy clientProxy, string method, object arg1, object arg2, object arg3, object arg4, object arg5, object arg6, object arg7, object arg8, object arg9, CancellationToken cancellationToken = default);
    public static Task SendAsync(this IClientProxy clientProxy, string method, object arg1, object arg2, object arg3, object arg4, object arg5, object arg6, object arg7, object arg8, CancellationToken cancellationToken = default);
    public static Task SendAsync(this IClientProxy clientProxy, string method, object arg1, object arg2, object arg3, object arg4, object arg5, object arg6, object arg7, CancellationToken cancellationToken = default);
    public static Task SendAsync(this IClientProxy clientProxy, string method, object arg1, object arg2, object arg3, object arg4, object arg5, object arg6, CancellationToken cancellationToken = default);
    public static Task SendAsync(this IClientProxy clientProxy, string method, object arg1, object arg2, object arg3, object arg4, object arg5, CancellationToken cancellationToken = default);
    public static Task SendAsync(this IClientProxy clientProxy, string method, object arg1, object arg2, object arg3, object arg4, CancellationToken cancellationToken = default);
    public static Task SendAsync(this IClientProxy clientProxy, string method, object arg1, object arg2, object arg3, CancellationToken cancellationToken = default);
    public static Task SendAsync(this IClientProxy clientProxy, string method, object arg1, object arg2, CancellationToken cancellationToken = default);
    public static Task SendAsync(this IClientProxy clientProxy, string method, object arg1, CancellationToken cancellationToken = default);
    public static Task SendAsync(this IClientProxy clientProxy, string method, CancellationToken cancellationToken = default);
}
\$\endgroup\$
5
  • \$\begingroup\$ Can you show the SendAsync method overloads? Does it apply params too? If it is, you may pass parameters as array there. \$\endgroup\$
    – aepot
    Aug 29 '20 at 13:44
  • 1
    \$\begingroup\$ @aepot , I updated the code \$\endgroup\$
    – Can PERK
    Aug 29 '20 at 14:43
  • \$\begingroup\$ Is connections a kind of Dictionary? I can suggest one more improvement then. \$\endgroup\$
    – aepot
    Aug 30 '20 at 14:27
  • 1
    \$\begingroup\$ Yes it is, please shot! \$\endgroup\$
    – Can PERK
    Aug 30 '20 at 17:04
  • \$\begingroup\$ Updated the answer. \$\endgroup\$
    – aepot
    Aug 30 '20 at 17:34
3
\$\begingroup\$

As for me, this code is fine. Maybe there's some Reflection dark magic available but this one will be anyway faster.

The only thing I can suggest here is optimizing out an async State Machine because there's no code after await and the only await in the method can be executed.

Also note that you lookup through the Dictionary twice, first time when you check key with .ContainsKey, second time when you get the value with the same key. It can be done with single lookup using Dictionary.TryGetValue() method.

Also I suggest to follow async naming policy suggested by Microsoft, and rename the method to InvokeNotificationAsync.

private Task InvokeNotificationAsync(string userName, string methodName, params object[] parameters)
{
    if (connections.TryGetValue(userName, out var connection))
    {
        var proxy = Clients.Clients(connection);
        switch (parameters.Length)
        {
            case 0:
                return proxy.SendAsync(methodName);
                break;

            case 1:
                return proxy.SendAsync(methodName, parameters[0]);
                break;

            case 2:
                return proxy.SendAsync(methodName, parameters[0], parameters[1]);
                break;

            default:
                throw new HubException($"No method was defined with {parameters.Length} parameters in Hub");
        }
    }
}

Also it can look a bit better with C# 8.0 syntax

private Task InvokeNotificationAsync(string userName, string methodName, params object[] parameters)
{
    if (connections.TryGetValue(userName, out var connection))
    {
        var proxy = Clients.Clients(connection);
        return parameters.Length switch
        {
            0 => proxy.SendAsync(methodName),
            1 => proxy.SendAsync(methodName, parameters[0]),
            2 => proxy.SendAsync(methodName, parameters[0], parameters[1]),
            _ => throw new HubException($"No method was defined with {parameters.Length} parameters in Hub")
        };
    }
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.