mclarkk / lifxlan

Python library for accessing LIFX devices locally using the official LIFX LAN protocol.
MIT License
503 stars 115 forks source link

How do people feel about returning error codes instead of throwing WorkflowExceptions? #113

Open mclarkk opened 6 years ago

mclarkk commented 6 years ago

It has been frustrating to me how much error-handling code I need to put in for the simplest scripts. I'd like to redesign this, but I want to get people's thoughts about the best way to go about it.

An alternative would be to return a C-style error code for those functions that currently throw errors...then it would be up to the user to check the error code if it was something that really mattered. Or, potentially, an optional argument could be added (some kind of "must-succeed" flag) that would throw an error if the code didn't succeed, but otherwise continue as usual.

I think it would also be good to expose the number of retry attempts and the timeout to users. Right now, to play with these two variables users currently have to modify the lifxlan code directly and then reinstall if they want to fiddle with these variables. I think exposing the default values and also providing per-function options would help a lot with those who struggle with slow networks or poor connectivity.

These would be somewhat substantial changes to make, and the first one could potentially impact existing codebases (the programs will still run, they just may not behave as expected). What do people think about these changes?

Rtyui commented 6 years ago

I think that adding the c-style return codes is the best solution. The 'must succeed' parameter could not be the most intuitive approach. Also as user having control over the error returned offers a lot more flexibility for the design.

NathanC commented 6 years ago

Coming from a more statically typed/functional background (mainly Scala), my initial reaction is that you'd want to return some sort of Option or Try result, but looking into it, it doesn't seem very python idiomatic (stumbled across this post).

So for what it's worth, I prefer consuming from libraries that throw rather than provide sentinel values/error codes-- if the conditions for throwing are documented, of course. If I'm repeatedly calling a method from this library that throws, in a context where I don't care if it fails, I'd just tend to wrap it in some attempt_to_foo method. And apparently there's a way to suppress exceptions as of Python 3.4, so a consumer of this library could just do with suppress(WorkflowException): do_something if they want. Sentinel values seem more complex and potentially dangerous (as things are more likely to fail silently).

Anyhow, just my 2 cents. Pretty rusty on Python, so I have no idea what the best practices are here. Thanks for writing this library, it's really cool and fun to use!

tacaswell commented 5 years ago

From the point of view of what 'feels pythonic' I prefer exceptions over error codes.