in reply to Code review request for my first module - WWW::PostiniAdmin

A few main points I'd change.

The name of the module and the hardcoded URI data could and should be generalized. The URI data might be part of the cconstructor arguments, or it could be loaded from a configuration file. That would allow reuse by many sites.

Second, you have commented out initializations for 'our @ISA' and others. Don't use our for that. @ISA, @EXPORT, @EXPORT_OK, etc. are package globals. Declare with use vars qw( @ISA @EXPORT @EXPORT_OK ); to keep strict happy.

In the line open DOMAINS, ">Domains.txt" or die #...;, you should use the 3-arg open since you require 5.6.0 anyway, and the 'Domains.txt' should be an absolute path taken from configuration, like the URI data. Remember that a module has even less control than a script over the current directory. Relative paths are made bad things by that.

It would have been good to post the code directly, between readmore tags. Your links may not always resolve.

After Compline,
Zaxo

Replies are listed 'Best First'.
Re: Re: My first module. I can't find any problems with it!
by afresh1 (Hermit) on May 03, 2003 at 02:25 UTC
    I like the idea of reading the URI data from a config file, but I would hate to confuse people by including one. Also, the sections for parsing the input are pretty specific to this site as well, so I am not so sure that pointing it at different URI's would be a Good Thing. I expect to possibly send this in to postini for them to distribute, but I don't see it ending up on CPAN so I think that the specific nature works

    I would probably have figured this part out the part about 'our @ISA' and the rest, but I am not using them, and I got my stubs from h2xs and that is what it generated, so I assumed they were correct. Silly me :-)

    Interesting, I haven't used the 3-arg version before. Kewlness. I will change it to open DOMAINS, '>', 'Domains.txt' or die and use that form in the future!

    Here is the full code of the module:

    Read more... (32 kB)
    andrew
[reply]
[d/l]
[select]

In Section Seekers of Perl Wisdom