Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Lack of Must / New on API #17

Closed
ibraimgm opened this issue Sep 30, 2020 · 2 comments · Fixed by #18
Closed

Lack of Must / New on API #17

ibraimgm opened this issue Sep 30, 2020 · 2 comments · Fixed by #18

Comments

@ibraimgm
Copy link

Hello!

I'm migrating from Google's UUID Package, and there is an specific detail that I find odd: the lack of Must and/or New on the API. These two functions are useful when you're quickly prototyping or when you DO want the entire world to blown up in case of errors.

These two functions have the following signature:

func Must (id string, err error) string // panics if err is not null
func New() string // returns a new id or panics

Is this feature acceptable? If so, I already made the changes locally on my machine and can send you a PR.

@matoous
Copy link
Owner

matoous commented Oct 4, 2020

Hi!

Thanks for suggestion.

Personally, I think we could add MustID and MustGenerate as we already have ID and Generate functions that return new id. This would be consistent with the existing API which I think is important.

New sounds good but with the current setup of this package it would be confusing (we already have Nanoid() and ID()). I think calling gonanoid.New() is clear and nice but I don't want to introduce yet another function. The only option would be to do a breaking change and release it as a version 2 but that seems like an overkill (or maybe not? 🤔).

Must is ok too but for the same reasons as mentioned above I don't think we should include it.

Overall, I like the suggestions and find the naming better than what we have now. In version 2 we could have:

func New() (string, error) // returns a new id
func Must() string // panics on error
func Generate(alphabet string, size int) (string, error) // new id from given alphabet
func MustGenerate(alphabet string, size int) string // new id from given alphabet, panics on error

I will think through it but I think that introducing version 2 with ⬆️ changes might actually make sense.

@ibraimgm
Copy link
Author

ibraimgm commented Oct 8, 2020

Is it acceptable to make MustID / MustGenerate right now in the current version? This allows the functionality to be available now borrowing time to eventually plan v2 wihout disruption.

matoous added a commit that referenced this issue Oct 8, 2020
Add MustGenerate and MustID functions that panic on error instead
of returning it.

Fixes: #17
matoous added a commit that referenced this issue Oct 8, 2020
Add MustGenerate and MustID functions that panic on error instead
of returning it.

Fixes: #17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants