gopack

gopack copied to clipboard

The packer functions call panic() when an overflow happens, which means the callers have to always setup a defer'ed recover() to potentially recover from such panics. It might be more idiomatic to have the packer type defined as type packer func(b []byte, v reflect.Value) error (i.e. have it return an error). What do you think?

Yeah, I've toyed with this idea a lot. The justification for having them be panics is that it's an error that happens when the program is incorrect, not one that can happen to correct programs (like a file read error, for example). This tends to be the distinction that is used to differentiate a panic from an error in the stdlib (see, for example, io.CopyBuffer, which panics if given a 0-length buffer). In this case it's a tad more subtle because:

• With normal primitive number types (uint8, int32, etc), nothing ca never panic, and instead a few things happen: if you overflow, then the type just has overflow behavior (it's part of the semantics, not an error condition of some sort); if you try assigning a value which is too large, then necessarily it's either a constant (which is a compiler error) or a different integer type (which requires a conversion, or else is a type error).
• If the user is accepting input from somewhere else (another package, the command line, etc), and using that input to generate values to pack, then the panicking behavior means they have to explicitly check all their inputs by hand first.

Given these considerations, I think the most reasonable thing to do might actually be to just have the behavior be wrap-around: if a value is out of the range that can be packed, it just wraps around like a normal machine integer. This would mean that if you wanted to avoid wrap-around, you'd still have to manually check (but that's already the case with the primitive number types), and it would also mean that there'd be no need to either check error values or recover from panics. Thoughts?

I'd rather not have it wrap around silently. Since the code already detects a "bad" value, I think erroring out is a sane thing to do. I generally agree with the distinction you outlined for panic vs error. Since the gopack code does the bounds checking already for me anyway, I was hoping to be lazy and avoid checking all the values myself and rely on gopack doing it – which is nice – hence why I was wondering whether we could return an error instead.

Hmmm... So I've never actually used gopack for anything, so maybe I don't have the best intuition about it. Is the code you're writing available so I can take a look at how you're using it?

I find it funny you've never used it for anything. I'm almost done with my prototype, I'll push the code to GitHub once I got something working and some tests written. I'll point you to the code then.

Hahah yeah, I just got the idea one Friday afternoon and powered through the implementation in a weekend; it wasn't out of necessity. Other than a few improvements I made off and on, it's basically just a hackathon project.