projectgoav / E.Deezer

Unoffical asynchronous Deezer .NET API
17 stars 11 forks source link

Refactoring #78

Closed PoLaKoSz closed 4 years ago

PoLaKoSz commented 5 years ago

I saw the new 5.X milestone and i thought i will ask can i propose some structural changes to hopefully make the future development more easy?

TODO

projectgoav commented 5 years ago

Yep, agree with a lot of these points.

Use .NET built-in HTTP compression

What do you mean by this? Some implementations of HttpMessageHandler support compression automatically which is used. If the given MessageHandler implementation doesn't, we fallback to providing it ourselves.

Add server-side Exception checking to all (GET, POST, DELETE) requests

What are you proposing here?

Remove internal interfaces which has only one implementation

I think only API objects are exposed have public interfaces. I'll need to review this a bit more.

PoLaKoSz commented 5 years ago

Use .NET built-in HTTP compression

Sorry, probably i wasn't completly clear. I found this SO answer to signal the server that this wrapper can understand gzip, deflate response. Relevant code: https://github.com/projectgoav/E.Deezer/blob/b2de5449f4739a04949ae66ffa1c7a0d55b52a17/E.Deezer/ExecutorService.cs#L176-L177 and https://github.com/projectgoav/E.Deezer/blob/b2de5449f4739a04949ae66ffa1c7a0d55b52a17/E.Deezer/ExecutorService.cs#L196-L219

Add server-side Exception checking to all (GET, POST, DELETE) requests

For example the https://github.com/projectgoav/E.Deezer/blob/b2de5449f4739a04949ae66ffa1c7a0d55b52a17/E.Deezer/DeezerClient.cs#L144 method only checks (if i remember correctly) for HTTP status errors, and not inside the json response which could contain the error property. Yes, there is the GetPlainWithError method but probably this is a bit redundant in my opinion because i would probably check every server json response for any Exception.

Remove internal interfaces which has only one implementation

For example the IDeezerClient is an internal interface.

projectgoav commented 5 years ago

WRT your first point: The library allows you to pass an instance of the MessageHandler. if you are writing a Xamarin app for example, you can pass in the native NSUrlSessionHandler on Mac and iOS which allows you take advantage of the features of that platform. Each handler works differently and the default way works by stripping the content type headers if the handler has handled the compression.

It tries to let .NET do as much as possible before falling back to the library handling the decompression.

Add server-side Exception checking to all (GET, POST, DELETE) requests

This really should be done! :)

PoLaKoSz commented 4 years ago

@projectgoav i am finished with the refactoring.