1
\$\begingroup\$

For some reason declaring ApiResult before checking if the answer is in a correct status doesn't look good. I wonder if there is any better way to check the status and return the result:

public async Task<ApiResult> GetCustomersByOutcode(string outcode)
{
    using (var client = new HttpClient())
    {
        ConfigureHttpClient(client);

        var response = await client.GetAsync(
            $"customer?q={WebUtility.UrlEncode(outcode)}");

        ApiResult result = null;
        if (response.IsSuccessStatusCode)
        {
            result = await response.Content.ReadAsAsync<ApiResult>();
        }
        return result;
    }
}
\$\endgroup\$
  • \$\begingroup\$ You never wrote which language this is in. Looks like C#? \$\endgroup\$ – MortenMoulder Mar 13 '16 at 11:01
  • \$\begingroup\$ It is indeed. Edited. \$\endgroup\$ – tribet Mar 13 '16 at 11:07
5
\$\begingroup\$

I suggest to remove the result variable. You can just return the result. You don't have to store it in a temporary variable:

if (response.IsSuccessStatusCode)
{
    return await response.Content.ReadAsAsync<ApiResult>();
}
else {
    return null;
}

In this case I would even use the ternary ? : operator:

return response.IsSuccessStatusCode
    ? await response.Content.ReadAsAsync<ApiResult>()
    : null; 

You could also write it like this:

if (!response.IsSuccessStatusCode)
    return null;

return await response.Content.ReadAsAsync<ApiResult>();
|improve this answer|||||
\$\endgroup\$
  • \$\begingroup\$ but return null is not a bad practice? could we get rid of it? \$\endgroup\$ – tribet Mar 13 '16 at 11:50
  • \$\begingroup\$ Well, you can make a special subclass of ApiResult to capture the failure case. Usually it is also an alternative to throw an exception. However, since you are in an asynchronous context, I recommend the former. \$\endgroup\$ – lex82 Mar 13 '16 at 12:08
  • \$\begingroup\$ @tribet I personally don't find return null; to be bad practice. Can't speak for everybody here but there are others who agree. \$\endgroup\$ – Rick Davin Mar 14 '16 at 21:15
0
\$\begingroup\$

What about using response.EnsureSuccessStatusCode() ? So it would be something like

response.EnsureSuccessStatusCode();
return await response.Content.ReadAsAsync<ApiResult>();
|improve this answer|||||
\$\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.