top | item 9890727

(no title)

nicksardo | 10 years ago

I strongly disagree.

The package author means the error will be nil when the secret is 32 characters long; he doesn't mean you can completely ignore checking it.

Regarding when to use panic: "The convention in the Go libraries is that even when a package uses panic internally, its external API still presents explicit error return values." http://blog.golang.org/defer-panic-and-recover

Panics should only be used in cases of programmer error or when the process is in an unrecoverable state.

discuss

order

nothrabannosir|10 years ago

> The package author means the error will be nil when the secret is 32 characters long; he doesn't mean you can completely ignore checking it.

But that's exactly what will happen. This comment will lead to ws, _ := gowork.NewServer("32 character secret"). Which is a worse situation than a panic(). I understand the normal rule for this, but "a foolish consistency..". In this case, the init value being 32 long can be compared to "the init value must be non-nil." A lot of non-error APIs will panic() if you pass nil, even in the stdlib.

Doing panic() here will lead to less room for error and confusion, in my opinion. Especially because the error is explicit and loud (panic()), it's worth considering breaking the rules for.

nicksardo|10 years ago

If a developer is frequently ignoring returned errors, then they have larger problems and it's their fault. Panic was not added to assist lazy programmers. Even though some API has solidified, an author could later add different return errors. Heck, if that function was calling other functions with returned errors, then the list of possible unique errors increases greatly. You should never ignore an err because you think you know the possible reasons.

The package author should modify the phrasing of that comment.

bcgraham|10 years ago

An idea is to include a function in the library called MustNewServer() that panics instead of returning an error. A usability perk of having single return is that the function can be chained, i. e.,

    ws := gowork.MustNewServer(key).AddParams(...)

jalfresi|10 years ago

As much as I hate chaining functions, I do like the idiom of declaring that a function may panic with the 'Must' prefix.

As long as there is an alternative function which returns errors :)

ryanskidmore|10 years ago

Thanks for the suggestion, i'll add something similar to this when i've got a bit of time. Cheers!

nothrabannosir|10 years ago

Great suggestion---I recant my original comment and second this.