kevbite / CompaniesHouse.NET

A simple .NET client wrapper for CompaniesHouse API
MIT License
37 stars 44 forks source link

Ideas for changing the CompaniesHouseClientResponse class #189

Open kevbite opened 2 years ago

kevbite commented 2 years ago

Currently, all method calls return a CompaniesHouseClientResponse<T> which is a wrapper around a Data object, however, recently we've added StatusCode, ReasonPhrase, and RetryAfter (https://github.com/kevbite/CompaniesHouse.NET/pull/182) so that clients can apply back off policies when calling endpoints on the API. The new feature was just added as an exception which isn't great for a consumer as it's not apparent how to use these features as it's hidden away in exceptions.

We're most likely going to introduce breaking changes when changing to use the new .NET serializer so it's most likely both updating the API surface with these changes. #188

This is an issue to discuss what we think the models should change to.

I like the idea of returning back a base class in which the client will need to switch on but then it does require some knowledge of which class types are return.

var result = await client.GetCompanyProfileAsync("10440441")
   switch
   {
       CompaniesHouseClientOkResponse<CompanyProfileResponse> ok => /* do something */,
       CompaniesHouseClientNotFoundResponse notfound => /* do something */,
       CompaniesHouseClientTooManyRequestsResponse tooMany => /* do something */,
       _ => /* do something */
   };

We could also have it so the base class has helper properties on it so that you'd not need to switch.

class CompaniesHouseClientResponse<TData>
{
    public TData => this switch {
       CompaniesHouseClientOkResponse<CompanyProfileResponse> ok =>ok.Data,
      _ => throw new InvalidOperationException("boom!")
   };
}
DavidBetteridge commented 2 years ago

I quite like the switch statement approach as it forces you to consider if the call was successful or not.