Naming

The author introduced a new root namespace: Handy::. This is a big mistake. All modules are supposed to be handy, so why announce that your module is handy? Clearly he doesn't understand CPAN. Exactly the same mistake was made with Dandy. Especially since this module is not handy, and not dandy.

Version numbering

Version numbers are important for many things. The distribution is version 1.306.2, but the actual $VERSION is 1.30_6. The $VERSION did not change since the previous release.

Changes

I always like to know what has changed since the previous release. But this Changes file only lists the initial version. Too bad.

Object orientation

Well, that's what the author seems to think. He used OOorNo, which he also wrote himself. A useless module that gives you your arguments without the first one if that's a class or object. This has nothing to do with object orientation, it's just some very strange and useless syntactic sugar: you can now use Package->function and $object->funtion instead of Package::function. Yay?

Documentation

Yeah, good question. Where is it? There is some pod in the module, but it tells you nothing more than the code does. It lists @ISA, @EXPORT, @EXPORT_OK, %EXPORT_TAGS and the available functions ("methods"). The functions are not documented. There's not even a hint about what arguments they expect. Or about what they return. If you want to use any of these "simple, reusable code routines", you will have to read the module's source.

So I did. It's horrifying. I'm more or less documenting the module now.

use_once

This function does nothing. Maybe it should have been called use_not. An empty block. What's the use?

isnum

This is shorthand for the combination of the next two functions. It passes @_ entirely (explicitly), but since isint and isfloat need a single argument, this one does too.

isint

This function takes one argument, and returns that argument if it matches ^\d*\z, undef if not (or not defined). This means a false value is being returned if you give it 0. I thought 0 was an integer.

I think the regex I used was too simple or something. Mister Butler tries to impress the reader with a nice tr, but he doesn't understand how it works.

$check !~ tr/0123456789/0123456789/cd ? $check : undef
[download]

As you can see, there's a double negation: !~ and /c. Useless obfuscation. And /d is being used to delete found but unreplaced characters. But since the explicitly written 0-9 is repeated in the RHS, nothing actually is deleted. Not that any deletion is needed: the variable is thrown away.

There's 5 lines of explanation comments about why underscore is not handled at all. Nothing is said about periods. I can live with 3.0 being an integer and it being not an integer, as long as the reason is documented.

There are some regexes in perlfaq4 to handle this kind of stuff, but I guess Tommy Butler never read the perlfaqs.

I'd probably use a simple int $foo == $foo.

isfloat

The first argument is used, all others are discarded. The function returns undef when it thinks the value you give it is not a floating point number, or the number itself when it thinks it is. This function too is loaded with tr vagueness.

If the stringified version of the value to check does not contain a dot, it's not a float. This means isfloat(1.0) returns undef, while isfloat("1.0") returns 1.0.

Any string ending in a dot, or having more than one dot is not a float. This means 1. is neither an integer nor a float, according this module. That also means it's not a number. Perl can handle this format, though.

Again, perlfaq4 has a much better solution.

Oh, I almost forgot to mention that because isint returns the number for integers, and because isnum uses a truth check for what isint returns, and because isfloat rejects strings without a period, this module doesn't regard zero as a valid number.

html_escape

(I'm disregarding unicode here.)

The author chose to split the string passed it into separate characters, and then iterate over that array to implement s/([\x21-\x2F\x3A-\x64\x91-\x60\x7B-\xFF])/'&#' . ord($1) . ';'/ge;.

That's right: non-printing characters are left alone. Note that of the 33 ASCII characters that this function substitutes for entities, only four actually need to be: &, <, > and ".

&# is written as \046\043, ; is written as \073. I guess this is just to impress people, to show off his l33tness.

HTML::Entities is a much better module for this, even though something as simple as s/([^\x20-\x7E])/'&#' . ord($1) . ';'/ge works fine.

isin

A temporary hash is used to see if the first argument is equivalent to any of the other arguments. The hash values are equal to the keys, what a waste of space. Besides, the entire list is copied a few times. All of this just to be able to write isin($foo, @bar) where grep $_ eq $foo, @bar would probably have sufficed.

If the first argument is false, this function returns undef. This means you cannot check if the list contains undef, an empty string or zero (string or number).

String comparison is used, because hash keys can only be strings. isin(5.0, @foo) will find 5 in @foo, but not 5.0. There's no way to get this function to use numeric comparison.

convert_size

This is used as convert_size($foo, 'bytes to megabytes'). It only supports bytes (/^by/), kilobytes (/^ki/) and megabytes (/^meg/), and cannot handle extra whitespace. The second word of the second argument is ignored.

This function made me extremely sad. Dear Tommy, a kilobyte is not 1000 bytes, it is 2**10 == 1024 bytes. A megabyte is not 1000000 bytes, it is 1024 * 1024 == 1048576 bytes. All computer users, but especially coders should know this!

utf8

I'm skipping this, because I know nothing about utf8. I assume the function is broken, but lack clue to prove it. Update - It has nothing to do with utf8. At all. This function is just a miserable attempt to re-implement what URI::Escape does best.

html_encode

This does what html_escape does, but with all characters. This function too splits, iterates and joins where a very simple regex would also have functioned (and have been much faster).

Please learn some Perl before uploading another module to CPAN. Make sure you understand what you write completely. If you do not understand something, ask for help. The Perl community is a friendly and helpful one (although this review is not at all nice). When you know Perl, you probably would not even think about creating a module like this.

Read what the CPAN guides say about communication. Post your module somewhere and request comments.

To PerlMonks readers

Downvote me if you want to. I see no reason to be nice to this author. He polluted CPAN, and didn't read perlfaqs, didn't read http://pause.perl.org/pause/query?ACTION=pause_04about. People like Butler give Perl and CPAN a very bad name.

If you're learning Perl, a good exercise is to read http://search.cpan.org/src/TOMMY/Handy-Dandy-1.306.2/Dandy.pm and find out where the bugs are. There are plenty and I expect even beginners to be able to find some.

Sorry if this ruined your Christmas.

Flame off.