speps / go-hashids

Go (golang) implementation of http://www.hashids.org
MIT License
1.32k stars 109 forks source link

Ignoring NewWithData error leads to runtime errors #34

Closed matthewvalimaki closed 7 years ago

matthewvalimaki commented 7 years ago

With introduction of NewWithData returning error vs panic there's now a chance of runtime error if the error is ignored. Say you have an alphabet with two same characters (causes error to be thrown) and you choose to ignore the error and continue with h.Encode() it will lead to runtime error because (h *HashID) is nil. Readme example by default ignores the error.

While of course it is bad practice to ignore errors I wonder if all exported functions needs check if h == nil. Only reason why I am thinking about such an implementation is because the library is small (easy to implement such check) and because the library can play a crucial part of web application setup. Rather than dying without error message it would be nice to have message saying why something failed.

speps commented 7 years ago

Rather than dying without error message it would be nice to have message saying why something failed.

So we're throwing an error because the user ignored the error in the first place? I'm not a fan of hand holding in libraries. I should probably fix the example though you're right that it doesn't handle errors.