Remove all usage of "log" about cron HOT 4 CLOSED

I'm also willing to do this as well. Currently there is some output in the tests from these panics that I would like to clear out.

I've also noticed (when developing on the package) that in some situations the panic handler will also catch stuff like index out of range, which only occurs when the code itself is incorrect and needs to be fixed. It also ends up hiding the stack trace when this happens. I would much rather known errors be returned as an error and have incorrect code panic without recovery. Or, at the very least, when a true panic happens also include the stack trace.

from cron.

@robfig I'd like to fix this by reworking all methods to return errors instead of nasty log+panic+recovery flow you currently have, are you ok with me doing so? It won't happen sooner than beginning of September when I'm back from PTO.

from cron.

You mean in parser.go? I'm a little adverse to that quantity of code churn, and I believe that panic is ok to use within a package, as long as it's not exposed to callers.

Here's a reference for that: https://github.com/golang/go/wiki/PanicAndRecover
"Within a package, however, especially if there are deeply nested calls to non-exported functions, it can be useful (and improve readability) to use panic to indicate error conditions which should be translated into error for the calling function"

I'm completely fine with replacing log.Panicf with a regular panic(fmt.Sprintf()) though! Seems like a mistake that it wasn't that way to begin with.

Thank you!

from cron.

Personally I prefer not to use panics at all, only in the situations when utterly necessary, using error return values, instead. I've seen this pattern several times all over the internet, including go's standard library. Additionally, I was thinking about completely replacing panics with errors returned from method, I'll see how it goes when actually working with the code, though 😉

from cron.