RoguePlanetoid / Spotify-NetStandard

Spotify API .NET Standard Library
MIT License
17 stars 2 forks source link

Feedback #2

Open hansmbakker opened 5 years ago

hansmbakker commented 5 years ago

Nice that you're creating this! I was wondering whether you considered https://github.com/reactiveui/refit to simplify creating your REST client?

RoguePlanetoid commented 5 years ago

Thanks - I'd based this client on the Groove Music SDK but anything I can do with it later might be worth considering as a later improvement using something like this!

RoguePlanetoid commented 5 years ago

The REST Library has been rewritten to improve the workflow, hopefully this makes it much easier to understand

hansmbakker commented 5 years ago

I meant that it might make it much easier for you to maintain this library and to add new functionality if you do not hand-write the REST Client code yourself. Also it might make your code much cleaner.

Refit just asks you to write a .Net interface and specify the REST method, parameters and url route to disclose an endpoint.

hansmbakker commented 5 years ago

More specifically, it would make a part of the contents in your Spotify.NetStandard/Spotify.NetStandard/Client/Internal folder redundant. I understand you started from the Groove Music SDK but I believe using Refit would be much simpler.

hansmbakker commented 5 years ago

Besides the point about Refit, I was wondering how you think about the circular dependency between SpotifyApi and SpotifyClient. Personally I find it very confusing and I think it could improve the code if there is one class that makes the REST calls and one class that might add some logic to the REST calls.

RoguePlanetoid commented 5 years ago

Good point about that circular dependancy - I'd intended that ISpotifyApi Interface to be a wrapper for the ISpotifyClient one but agree it is confusing and could refactor this to remove that

hansmbakker commented 5 years ago

Nice :) I was wondering whether you've had the chance to look at Refit, and what you think of it?

My recommended approach would be: