Anonymous Monk has asked for the wisdom of the Perl Monks concerning the following question:

Ihave some questions. I want to make some basic module that could one day go on cpan. Please review my example module, which is not the actual code, but a cut down example of how i do things. 
package MyModule;

use version; 
use warnings;
use strict;
use Carp;

use CGI;
use base qw( Exporter );
our $VERSION     = qv ('0.0.1');
our @ISA     = qw (Exporter);
our @EXPORT_OK   = qw (new);
our %EXPORT_TAGS = ( DEFAULT => [qw (new) ],
            All  => [qw (new bar browserEnv)] );

sub new{ 

    my $class = shift;
    my $self = {};
    my %arg = @_;

    $self->{ResourcePath} = $arg{ResourcePath};
    $self->{AssignedVal} = $arg{AssignedVal};

    bless ($self, $class);

    return $self;



}



sub bar{

    my $self = shift;

    print "<p>Testing, called sub 'bar' within MyModule</p>";
    print "<p>Testing, ResourcePath: ".$self->{ResourcePath}."</p>";
    print "<p>Testing, AssignedVal: ".$self->{AssignedVal}."</p>";
    return;

}



sub browserEnv(){

    my $self = shift;

    my $cgi = new CGI;

    if($cgi->http('HTTP_REFERER')){

        print "<p>Testing, there was a referer: ".$cgi->http('HTTP_REF
+ERER')."</p>";

    }else{

        print "<p>Testing, there was <b>no</b> referer</p>";

    }

    return;

}    

1;

[download]
Example script 
#!/usr/bin/perl

use strict;
use warnings;

use CGI;
use MyModule;

my $cgi = new CGI;
print $cgi->header;
print "<html>\n<body>";
print "<p>Testing module</p>";

my $foo = MyModule->new(
            ResourcePath    => '/opt/app/res',
            AssignedVal     => '50',
            );

$foo->bar();
print "<p>Testing we can access object, print value of module paramete
+r: $foo->{AssignedVal}</p>";
print "<p>Testing we can change an value (AssignedVal=100):<p>";
$foo->{AssignedVal}=100;
$foo->bar();
$foo->browserEnv();
print "</body>\n</html>";

[download]

My questions.

Basically, is this how things should be done? I would like to here if I am doing things the right way. Should the module need use CGI even if the calling script is using it already, does this load it twice? Am i altering the value of AssignedVal correctly?

Thank you for your time

Replies are listed 'Best First'.
Re: Question about making modules.
by dragonchild (Archbishop) on Jul 06, 2008 at 15:25 UTC
    Some comments, not necessarily in any order:
    • Put in a required minimum Perl version. Based on what you have, 'use 5.006_000;' is probably appropriate.
    • Don't use version. Don't use v-strings.
    • Don't export new(). In fact, if you're OO, there's very little you should ever have to export.
    • If you're going to have people pass in key-val pairs to new(), make sure you check you have an even number. Otherwise, my %args = @_; can throw a warning.
    • use base sets @ISA and does a "use Exporter;". Don't replicate its functionality.
    • You have a lot of "Testing" lines. Aren't you going to use Test::More?
    • Sounds like you're doing a browser thing. Maybe your functionality would be better managed as a plugin to an existing framework?

    Have you looked at how CPAN modules are written? Take a look at what they do - there's a style that's evolved over the years in order to handle the vast array of installation targets.

    My criteria for good software:
    1. Does it work?
    2. Can someone else come in, make a change, and be reasonably certain no bugs were introduced?
[reply]
[d/l]
      Put in a required minimum Perl version.

      Gah. Please don't. Don't proclaim "This requires Perl version X" unless you actually know that it really does require it. And if you do go to the effort of figuring out that a particular version is required, then please be quite specific in describing why that particular version is required (at the least).

      I'd much rather get an error that points right at the thing that isn't supported with my version of Perl than be told "The author of this module has decided to prevent this module from even attempting to run on your version of Perl but absolutely no insight as to why was provided." That's just rude or worse.

      Update: It'd be nice if "use 5.006_001('open my $fh');" were supported so that when an author provides insight we don't have to go hunting for it.

      - tye        

[reply]
        I'm with you there. Being able to specify minimum feature requirements vs. minimum version is a much nicer solution, and not just for Perl versions.

        In the meantime, specifying a minimum Perl version is better than failing randomly after installation. To me, "use 5.006_000;" is important because of the number of times I've moved files from Pure-Perl modules around and hit random bugs.

        My criteria for good software:
        1. Does it work?
        2. Can someone else come in, make a change, and be reasonably certain no bugs were introduced?
[reply]
      Don't use version. Don't use v-strings.

      Can you explain or give a link to an explanation why not?

      What to use instead?

[reply]
        v-strings weren't supported prior to 5.6 and the support for them changed in 5.8 and they were removed in 5.10. So, do you want to use something with that kind of track record?
        My criteria for good software:
        1. Does it work?
        2. Can someone else come in, make a change, and be reasonably certain no bugs were introduced?
[reply]
[reply]
Re: Question about making modules.
by cdarke (Prior) on Jul 06, 2008 at 18:58 UTC
    If your module is going onto CPAN then you should also have POD at the end which documents the class - that is one place where your example script can go, as well as a description of the methods. See perlpod, and any of the modules in the base.

    Should the module need use CGI even if the calling script is using it already, does this load it twice?
    As a general principle your module should load and compile without assuming the caller has done anything, it should be a "black box". If the caller (or any other modules) duplicate loading modules then that is OK, they are not loaded more than once - just do not assume any particular load order.
[reply]
Re: Question about making modules.
by EvanCarroll (Chaplain) on Jul 06, 2008 at 19:30 UTC
    • Use module-starter to create the module module-starter --mi --module="MyModule" --author="Anonymous Monk"
    • Use Module::Install, on all new modules
    • Use Test::More if your going to write tests
    • Learn Moose (it would save you from having to write new()
    • Don't ever use Exporter on an OO module
    • You're better off passing HashRefs than arrays ->new({foo=>bar}) rather than ->new(foo=>bar)
    • Don't use indirect object notation 'stupid Wizard nonsense' Wizard->stupid(nonsense)


    Evan Carroll
    I hack for the ladies.
    www.EvanCarroll.com
[reply]
[d/l]
Re: Question about making modules.
by pc88mxer (Vicar) on Jul 06, 2008 at 20:02 UTC
[reply]
Re: Question about making modules.
by ysth (Canon) on Jul 06, 2008 at 19:31 UTC 
        if($cgi->http('HTTP_REFERER')){

        print "<p>Testing, there was a referer: ".$cgi->http('HTTP_REF
+ERER')."</p>";

    }else{

        print "<p>Testing, there was <b>no</b> referer</p>";

    }

    [download]
    Note that the correct spelling is "referrer". That is how it should be spelled in English text, despite the unfortunate misspelling in the HTTP standard that we are all stuck with forever.
    --
    Online Fortune Cookie Search
[reply]
[d/l]
Re: Question about making modules.
by toolic (Bishop) on Jul 07, 2008 at 13:58 UTC
[reply]

Back to Seekers of Perl Wisdom