delannoyk / SoundcloudSDK

A Client for Soundcloud's API written in Swift!
MIT License
87 stars 27 forks source link

Static func Session.login should go to Soundcloud.login #20

Closed larsblumberg closed 8 years ago

larsblumberg commented 8 years ago

This Soundcloud SDK has a great API and source code – I've just started using it and it seems exceptionally well designed + implemented to me. It makes use of advanced Swift features such as generic structs that make the code a pleasure to read and use.

I understand that class Soundcloud holds a static variable for the OAuth session, named session. Since class Soundcloud is the owner of that static variable, I would suggest to move the static login function (and related login/logout functions) from Session to Soundcloud itself. I argue that Session.login and Session.destroy shouldn't access Soundcloud's static variable session. As said, Soundcloud should manage this variable itself. This also makes it easier to read:

Soundcloud.clientIdentifier = "..."
Soundcloud.clientSecret = "..."
Soundcloud.reditURI = "..."
Soundcloud.login(...)
Soundcloud.logout(...) // or alternatively Soundcloud.session?.destroy()

instead of

Soundcloud.clientIdentifier = "..."
Soundcloud.clientSecret = "..."
Soundcloud.reditURI = "..."
Session.login(...)
Session.destroy(...)

What are your thoughts on this @delannoyk?

delannoyk commented 8 years ago

Hi @larsblumberg You make a very good point and I agree with you. Going to refactor this.