discordgo
discordgo copied to clipboard
Rethink `New` function API
Proof of concept implementation in #651
Problem
The New
function does not express its intent in its type. it is also very unintuitive to use.
func New(args ...interface{}) (s *Session, err error)
The type signature tells the user nothing, i think this API was picked because of different ways to authenticate. I think this is a bad idea and it is more intuitive to use a struct.
Solution
Add a new more intuitive struct based API. something along the lines of this
dg, err := discordgo.WithAuth {
Email: "[email protected]",
Pass: "foobar",
Token: "abc123",
Bot: false,
}.Verify()
I'm using a method here to prevent me from having to type indiscordgo
again. you could also do it like this
dg, err := discordgo.New(discordgo.WithAuth{
Email: "[email protected]",
Pass: "foobar",
Token: "abc123",
Bot: false,
})
In my opinion both are better then the current API, and could be added while remaining backwards compatible.
If this gets approved and i have time i'll implement this (it should not be that hard).
I like where this is heading, an API where you can pass in a WithAuth option, or WithToken option sounds great to me
On Sun, May 26, 2019, 7:25 AM Ulisse mini [email protected] wrote:
Problem
The New function does not express its intent in its type. it is also very unintuitive to use.
func New(args ...interface{}) (s *Session, err error)
The type signature tells the user nothing, i think this API was picked because of different ways to authenticate. I think this is a bad idea and it is more intuitive to use a struct. Solution
Add a new more intuitive struct based API. something along the lines of this
dg, err := discordgo.WithAuth { Email: "[email protected]", Pass: "foobar", Token: "abc123", }.Verify()
I'm using a method here to prevent me from having to type indiscordgo again. you could also do it like this
dg, err := discordgo.New(discordgo.WithAuth{ Email: "[email protected]", Pass: "foobar", Token: "abc123", })
In my opinion both are better then the current API, and could be added while remaining backwards compatible.
If this gets approved and i have time i'll implement this (it should not be that hard).
— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/bwmarrin/discordgo/issues/649?email_source=notifications&email_token=AALVLASQRO7JCOLPWYGQ5GTPXKMWRA5CNFSM4HPWY52KYY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4GV4VRUQ, or mute the thread https://github.com/notifications/unsubscribe-auth/AALVLAVJJ36XO5VYXNSSGHTPXKMWRANCNFSM4HPWY52A .
-1, You don't always need an email, a password and a token. I think the current system is fine as it is.
Luckily, since the current New
function takes in an interface{}
, you can add this without breaking, and I'm all up for that.
@diamondburned If you don't specify a field then it will be set to its zero value, exactly what we want.
for example, not specifying the Bot
field would leave it to a default of false
not specifying an email and password would leave them to ""
and we could check for that in the New
/ Verify
functions
Personally, if we're to break the current API on everyone - I'm more in favor of dropping the Email/Pass entirely and just accepting a single token string that we pass along to Discord. It's a closer match to the goal of just being a wrapper around Discord's API and it's simple and easy to use.
The only reason this hasn't been done is because of not wanting to break the API on everyone for such a minor thing. Now with modules and versions and such, I do feel better about making breaking changes to help clean up this library.
I agree with @bwmarrin, it would also make the API much simpler, something like
func New(token string) (*Session, error)
would be desirable
You could also remove the error return value entirely in that case, since currently it's only used for paramater validation and fetching token from email/pass, if it takes only token no error would be returned.
My PR should add something like this without any breaking changes: https://github.com/bwmarrin/discordgo/pull/725