cpan you have to see

Gareth Harper spansh+london at gmail.com
Wed Dec 12 17:45:05 GMT 2012


On 12 December 2012 17:05, Alexej Magura <perlook at cpan.org> wrote:
> Okay, allow me to clarify what the TrueFalse module that I wrote is trying
> to emulate.  It's trying to emulate the 'true' and 'false' user commands
> available under Linux.
>
> Haven't you ever done something like this in Unix Shell?
>
> while true; do ls /var/log/; sleep 5s; clear; done
>
> The statment 'true' in this example, as far as I know, only returns true
> and that's it.  It may not look very useful, but it can be useful when
> you just need to do something and just want to write 'Just because I
> said so, keep doing A until I say stop.'
>
> I'm sorry if all of you think that my modules are poorly written, but if
> you want me to take you seriously, then say something productive for a
> change, that is make some suggestions (I'm open to suggestions.)


Without commenting on the function of the modules (I personally
wouldn't use them, but I can see what you're trying to accomplish).
Style/function/speed wise there certainly are a few areas which you
may want to address.  I'll explain some of the more obvious ones here,
though there are several other things you may want to look at.  I see
several other people have volunteered help as well.

First, you will probably want to look at "strict".  This will turn
strict mode on in Perl which will flag up possible problems in your
code (such as undeclared variables) and many other issues.  You can
read more here

Also there appears to be a lot of copied and pasted code inbetween
your modules, you should try to have one library providing the
function and then simply use that library in the places you need to
use it.

http://perldoc.perl.org/strict.html

Traditionally most CPAN modules come with a test suite which allows
both the author and anyone using the libraries to ensure they are
working correctly.  It also means if there are bugs found, then you
can simply create a test case to prove the bug, then fix it and be
sure it will never return.  There are many different libraries you
could use to accomplish this, but I find this one easy to use (it even
comes with Perl as standard).

http://search.cpan.org/~mschwern/Test-Simple-0.98/lib/Test/More.pm

If you create your module framework with something like the module
below (there are several other options), then you will have the entire
module skeleton laid out for you (including a basic test suite, and
all the documentation/MANIFEST files) and you simply need to add the
test cases and the code (and edit the documentation files).

http://search.cpan.org/~jkeenan/ExtUtils-ModuleMaker-0.51/lib/ExtUtils/ModuleMaker.pm


Looking at the modules individually:

https://metacpan.org/source/PERLOOK/Flexible-Output-Printer-0.4.5/lib/Printer.pm#PFlexible%3A%3AOutput%3A%3APrinter

All of these functions return a blessed anonymous empty hash.  In Perl
bless is used for object oriented programming, to link a particular
variable to a particular package, there is no reason in your modules
to use bless at all.  You can read more about bless here.

http://perldoc.perl.org/functions/bless.html

If your intent was to return a true value (which in this case, you
have done), then you can simply "return 1;".


https://metacpan.org/source/PERLOOK/Ez-Tf-0.1.2.2/lib/Tf.pm#PEz%3A%3ATf

Your "True" function here actually returns 0, which in Perl is treat
as false (undefined, empty string and 0 are the various false values),
you probably want to return 1 here.

Likewise your False and false functions simple return.  Now this will
be treat as false, however it would probably be better to "return 0;"
such that anyone using your code doesn't get undefined values errors.

Your true function has a "return 'True'", which IS a true value,
however I suspect you meant to call your True function, so you should
simply "return True".

As a sidepoint, having functions named the same, differing only in
case, but doing different things is generally frowned upon, it makes
code using them harder to read and easier to make mistakes in.



https://metacpan.org/source/PERLOOK/Ez-Printer-0.0.3.1/lib/Printer.pm#PEz%3A%3APrinter


Your putc function will exit any program that uses it, if you really
were wanting to throw a critical error, you should use the "die"
command, however the normal method would be to simple return 0 (or
false).  You could also look into using Error
(http://search.cpan.org/~shlomif/Error-0.17019/lib/Error.pm) to add
exceptions and error throwing.

Thanks

Gareth


More information about the london.pm mailing list