search

infobip / infobip-api-csharp-client

Infobip API client library in C#, distributed as a NuGet package.
https://www.infobip.com/docs/api
MIT License
11 stars 17 forks source link

Suggested refactoring #30

Open BluMichele opened 7 months ago

BluMichele commented 7 months ago

Hi there,

I was working on subscriptions for which there is no API class like SendEmailApi and I noticed that the following logic si repeated multiple times in each API class

// authentication (APIKeyHeader) required
if (!string.IsNullOrEmpty(Configuration.GetApiKeyWithPrefix("Authorization")))
    if (!localVarRequestOptions.HeaderParameters.ContainsKey("Authorization"))
        localVarRequestOptions.HeaderParameters.Add("Authorization",
            Configuration.GetApiKeyWithPrefix("Authorization"));
// authentication (Basic) required
// http basic authentication required
if (!string.IsNullOrEmpty(Configuration.Username) || !string.IsNullOrEmpty(Configuration.Password))
    localVarRequestOptions.HeaderParameters.Add("Authorization",
        "Basic " + ClientUtils.Base64Encode(Configuration.Username + ":" + Configuration.Password));
// authentication (IBSSOTokenHeader) required
if (!string.IsNullOrEmpty(Configuration.GetApiKeyWithPrefix("Authorization")))
    if (!localVarRequestOptions.HeaderParameters.ContainsKey("Authorization"))
        localVarRequestOptions.HeaderParameters.Add("Authorization",
            Configuration.GetApiKeyWithPrefix("Authorization"));
// authentication (OAuth2) required
// oauth required
if (!string.IsNullOrEmpty(Configuration.AccessToken))
    localVarRequestOptions.HeaderParameters.Add("Authorization", "Bearer " + Configuration.AccessToken);

// make the HTTP request

If I might, I suggest creating a new method like AddAuthenticationHeaders in ApiClient and calling it at the beginning of ExecAsync, you are already setting things up from configuration in that location, so why do half the job in ApiClient and half in each method of each API class, move all there 🤷🏻‍♂️.

BluMichele commented 7 months ago

Also, the code will never enter the third block of if-if-add which is identical to the first.

if (!string.IsNullOrEmpty(Configuration.GetApiKeyWithPrefix("Authorization")))
    if (!localVarRequestOptions.HeaderParameters.ContainsKey("Authorization"))
        localVarRequestOptions.HeaderParameters.Add("Authorization",
            Configuration.GetApiKeyWithPrefix("Authorization"));
ib-plivaja commented 7 months ago

Hi @BluMichele,

thank you for suggested updates and for using our library. We are planning to fully refactor this library in the next major version release your suggestions will be taken into account. We'll keep you informed.

Kind regards, Petra