3
\$\begingroup\$

This code simply sends a HttpRequest message and, if it comes back Unauthorized, gets a new login and tries again.

This code is meant to deal with the moment a JSON Web Token goes out of date. In this system the client is notified of this by having its request become Unauthorized, it then picks up a new Token by logging on again. Hiding away that Unauthorized code, it continues to message.

Two things to note:

  1. A HttpRequestMessage has to be recreated every time it is used.

  2. I have a need to recreate the client to pick up the new JWT token

(Edit: Actually can just go in and change some properties for client, I assume it would be worth changing?)

Basically my question is: How unpleasant is my use of delegates and what lengths should I go to change this code, if any?

private async Task<HttpResponseMessage> SendToFooAsync(Func<HttpClient> client, Func<HttpRequestMessage> requestMessage)
{
    var response = await client().SendAsync(requestMessage());

    if (response.StatusCode == HttpStatusCode.Unauthorized)
    {
        //Re-authenticate, recreate client with new token, and retry
        if (await authService.LoginAsync())
        {
            response = await client().SendAsync(requestMessage());
            Log.Log("Unauthorized code received: Re-Authentication attempt succeeded");
        }
        else
        {
            Log.Log("Unauthorized code received: Authentication attempt has failed");
        }
    }
    return response;
}

Simplest code that calls it:

HttpResponseMessage result = await SendToFooAsync(InitialiseHttpClient, () => InitialiseRequest(HttpMethod.Post, path));

Most complex code that calls it

Func<HttpClient> client = () =>
            {
                var httpClient = InitialiseHttpClient();
                httpClient.DefaultRequestHeaders.Accept.Add("some json stuff");
                return httpClient;
            };

HttpResponseMessage result = await SendToFooAsync(client, () =>
{
    var message = InitialiseRequest(HttpMethod.Post, path);
    message.Content = new StringContent("some json stuff");
    return message;
});

Where InitialiseRequest and LoginAsync are my own methods outside the scope of this review. Hopefully their use is obvious here.

\$\endgroup\$
  • \$\begingroup\$ BTW, the code you post here should be your real code (no "foo"s or "some stuff"s), because we want to be able to review all aspects of your code. I think it doesn't matter much here, but it's something to keep in mind for the next time. \$\endgroup\$ – svick Aug 2 '14 at 21:25
3
\$\begingroup\$

How unpleasant is my use of delegates and what lengths should I go to change this code, if any?

If you want the caller to provide some small piece of code, then a delegate is exactly the right way to do it, it's not unpleasant at all.

But looking at your two usage samples, it looks like they have some code in common (calling InitialiseHttpClient and InitialiseRequest), so it might make sense to change the signature to something like:

private async Task<HttpResponseMessage> SendToFooAsync(
    HttpMethod method, string uri,
    Action<HttpClient> clientAction = null,
    Action<HttpRequestMessage> requestMessageAction = null)

The two usage samples would then look like this:

SendToFooAsync(HttpMethod.Post, path);

SendToFooAsync(
    HttpMethod.Post, path,
    client => client.DefaultRequestHeaders.Accept.Add("some json stuff"),
    message => message.Content = new StringContent("some json stuff"));

Also, I don't like the way you're naming your delegates. Func<HttpClient> is not HttpClient, so it shouldn't be called just client. Something like clientFunc would be more accurate.

\$\endgroup\$
  • \$\begingroup\$ normally when you have a Func you suffix the identifier with "Factory", i.e httpClientFactory for example \$\endgroup\$ – Ahmed Alejo Jan 19 '16 at 17:00

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.