Async wrapper around public API

Tear it apart. I'm mostly concerned around the appropriate use of ConfigureAwait(false), and possible poor handling and duplication of the HttpClient in Invest and Get.

Additionally, I'd also like thoughts on the fact that for usage of these methods, one must call .Result to get the actual results. Is this standard for building such a wrapper or is there another way I should achieve this?

```
using System;
using System.Collections.Generic;
using System.Net;
using System.Net.Http;
using System.Net.Http.Headers;
using System.ServiceModel;
using System.Text;
using System.Threading.Tasks;
using Newtonsoft.Json;

namespace ProsperAPI
{
public class ProsperApi
{
private readonly string _username;
private readonly string _password;
private readonly string _apiBaseUrl = "https://api.prosper.com/v1/";
private AuthenticationHeaderValue _authenticationHeader;

#region Constructors

public ProsperApi(string username, string password)
{
_username = username;
_password = password;
_authenticationHeader =
new AuthenticationHeaderValue(
"Basic",
Convert.ToBase64String(
Encoding.UTF8.GetBytes(
string.Format("{0}:{1}", _username, _password))));
}

public ProsperApi(string username, string password, string baseUrl) : this(username, password)
{
_apiBaseUrl = baseUrl;
}

#endregion

public async Task Authenticate()
{
if (String.IsNullOrEmpty(_username))
throw new ArgumentNullException("_username", "Credentials are not set");
if (String.IsNullOrEmpty(_username))
throw new ArgumentNullException("_password", "Credentials are not set");

try
{
// The account call will fail out if credentials are incorrect, thus

There's something odd about your constructors - normally chained constructors call the constructor with the most parameters, not the other way around:

public ProsperApi(string username, string password, string baseUrl) 
    : this(username, password)

public ProsperApi(string username, string password)

Should be:

public ProsperApi(string username, string password)
    : this(username, password, null)

public ProsperApi(string username, string password, string baseUrl)

_apiBaseUrl will be null anyway when you call the 2-parameter constructor. The idea is that you write - and maintain, one constructor body.

This would be it:

public ProsperApi(string username, string password)
    : this(username, password, null)
{ }

public ProsperApi(string username, string password, string baseUrl) 
{
    _username = username;
    _password = password;
    _apiBaseUrl = baseUrl;

    _authenticationHeader = 
        new AuthenticationHeaderValue(
            "Basic",
            Convert.ToBase64String(
                Encoding.UTF8.GetBytes(
                    string.Format("{0}:{1}", _username, _password))));
}

With only 1 constructor doing all the construction work, you don't really need a #region here; regions are seldom ever really needed anyway, and more often than not their usage is a smell: if you need to "sectionize" a class, it's likely doing too many things. Now a Constructors region is different, it's more like a comment that says what the code already say.

Besides, you can collapse the constructor bodies, leaving only the signatures visible in the IDE; that's possibly more descriptive than any comment you can write there.

I like your private fields, private readonly fields assigned in the constructor are great. Why isn't _authenticationHeader readonly as well? If it's only meant to be assigned by the constructor, and not tampered with, then it should be made readonly.

In the Authenticate method, your guard clause is a bit weird:

if (String.IsNullOrEmpty(_username))
    throw new ArgumentNullException("_username", "Credentials are not set");
if (String.IsNullOrEmpty(_username))
    throw new ArgumentNullException("_password", "Credentials are not set");

You're not validating the _password (you're checking for a null or empty _username write) - nice copy+paste error you got here. But nonetheless, the goal of a guard clause is to fail early, and with _username and _password being readonly and only assignable in the constructor, wouldn't it make more sense to throw at construction time, if you're going to pass illegal constructor arguments that will make the object fail later, might as well throw right away and refuse to create an instance with null/empty credentials.

Other than by the method's name, it's not very clear why Authenticate needs _username and _password to be set, one has to trace all the way down to HttpClientSetup() to see _authenticationHeader in use, with the actual method call that would probably fail if _username or _password was null or empty. Preventing null/empty credentials at construction would ensure that the instance is always in a state that allows it to make these calls without worrying about the username and password values.

You're storing username and password as private readonly fields, but in reality you don't need to keep either - you only need them to construct the _authenticationHeader. I wouldn't keep that data in the instance, it's not needed. I'd change your constructor to this:

public ProsperApi(string username, string password, string baseUrl) 
{
    if (string.IsNullOrWhiteSpace(username) || string.IsNullOrWhiteSpace(password))
    {
        throw new ArgumentException("Username and password cannot be null or empty.");
    }

    _apiBaseUrl = baseUrl;

    _authenticationHeader = 
        new AuthenticationHeaderValue(
            "Basic",
            Convert.ToBase64String(
                Encoding.UTF8.GetBytes(
                    string.Format("{0}:{1}", username, password))));
}

Using string.IsNullOrWhiteSpace over string.IsNullOrEmpty will also refuse to take " " as a valid user name or password.

I went with an ArgumentException, because ArgumentNullException should be specifically for when an argument is null when it shouldn't be - and in this case the argument being null isn't the only way to trip the guard clause, so I'd rather throw a more general exception than a potentially lying/confusing specific exception - there doesn't need to be two separate checks, since the exception message will be the same either way.

Lastly, I like how you've named [almost] everything, except the async methods should, by convention, be appended with the word Async as a suffix, like:

public async Task> GetNotesAsync()
public async Task GetAccountAsync()
public async Task> GetListingsAsync()
private async Task GetAsync(string url)

Not

StackExchange Code Review Q#55647, answer score: 9