Re: [code-review] Bencode.pm

Caleb Epstein wrote:

> I'm soliciting comments on the following Perl module,
> Bencode.pm, which implements in Perl "bencode"
> serialization and deserialization routines.

What, no POD? Also, what's that #! -w stuff doing at the top of the module?
AFAIK shebang switches only apply to scripts, not subsequently loaded
modules.

Also, I personally prefer to only export symbols if they're explicitly
specified. Even when I use modules that export stuff by default, I tend to
fully-specify function names in production code (as opposed to quick and
dirty hacks), because it makes it easier to work out where that function was
defined.

As for @+: 

> substr ($x, $f) =~ /^(0|-?[1-9][0-9]*)e/;
> return int ($1), $f + $+[0];

Why say $+[0] rather than length($1)+1? (I haven't benchmarked the two.)
length($1)+1 is more obvious, and more backwards-compatible, to my mind.

I also dislike the preponderance of one-character variable names. We're no
longer using 1980s BASIC where only the first character of the variable
mattered, and RAM is cheap. We can afford to give more descriptive names to
variables.

Inconsistency: why fatal errors in decode_int, decode_string and decode_dict
but not in decode_list?

Checking whether something is an integer:
> $x =~ /^(0|-?[1-9]\d*)$/

Is that quicker and/or more reliable than int($x) eq $x? (Again, I haven't
benchmarked.) Can you get integers reported in scientific notation?

For that matter, the decoding functions get passed a buffer and an offset.
It's not clear how large this buffer is going to be, but given that this is
a BitTorrent client I imagine buffers are going to be fairly large. Do you
want to hold all of that in memory? Is it more efficient to call substr on a
scalar, or to read from a filehandle? (Which could well be a socket, a
blessed scalar or something else fancy.)

Sam
-- 
Home page: http://www.illuminated.co.uk/