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\$
2
  • \$\begingroup\$ You never wrote which language this is in. Looks like C#? \$\endgroup\$ Mar 13, 2016 at 11:01
  • \$\begingroup\$ It is indeed. Edited. \$\endgroup\$
    – tribet
    Mar 13, 2016 at 11:07

2 Answers 2

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>();
\$\endgroup\$
3
  • \$\begingroup\$ but return null is not a bad practice? could we get rid of it? \$\endgroup\$
    – tribet
    Mar 13, 2016 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, 2016 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, 2016 at 21:15
0
\$\begingroup\$

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

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