The first comment you are going to get on the c.l.p.modules is "Use a more descriptive name".

Also, wrapping it up with a few tests and in a neat installation package will help people use it more. cpan installs are preferred over copying a file.

I'll look at the module and give you some feedback. Is there good documentation with your contact info?

J. J. Horner
Linux, Perl, Apache, Stronghold, Unix
jhorner@knoxlug.org http://www.knoxlug.org/

Might I suggest OhNo? <G> -- theorbtwo -- http://www.rtweb.net/theorb/

At any rate, for your information, this would be more appropriately placed in 'Craft'. Meditations is an area for monks to come and reflect upon life, the universe, and Perl. Read some of the other entries in this section for examples.

Good luck, and when you're ready to put together a full, official package you should look up MakeMaker. :)

Alakaboo
I am the lurker that spams in the night.

Good idea for a module, not been done before though? says he going to have a look round CPAN.........

I don't see it on CPAN yet, but gave a quick look at the package from the link above. Quickly, I see you have use strict commented out.. why? You later have no strict 'refs'. Also, please do not put anything on the CPAN which has your email address as a default value. You should not make people set (or unset) default values within the module, ie:

my $def_smtp         = "your.smtp.com";    #<--IMPORTANT!  Set this!
my $def_email         = 'thomasoniii@yahoo.com';
my $def_return        = 'thomasoniii@yahoo.com';
my $def_subject     = "Ye Gods!  An error!";
my $def_domain        = "smtp.com";        #<--IMPORTANT!  Set this! I
+ mean it!
my $def_log_file     = "error.log";
[download]


These things should be done ONLY by passing a hash (I would use a hash) when use'ing the module (or in a new() method, which you don't need to have), or with methods (like Carp::Notify::set_smtp('smtp.me.com'), or Carp::Notify::set_defaults({smtp => 'me.smtp.com', email => 'foo@me.com'}), etc...).
Please change this.


my $paddr = sockaddr_in(25, $remote_address);
[download]


How do you know it is 25?


sub today {
...
}
[download]


I like POSIX for things like this, some don't.

sub error { undef};
[download]


I would have finished this sub before putting on CPAN, personally. You could $Carp::Notify::errlog = 'file' or STDERR (etc...), if Win32 you could log to the Event Log, or make error() a callback to a user defined sub to handle the errors as they wish.

Just some comments from a quick review.

Cheers,
KM

I don't see it on CPAN yet, but gave a quick look at the package from the link above. Quickly, I see you have use strict commented out.. why? You later have no strict 'refs'.

I developed the module using strict to make sure that it behaves itself. But once it's done and functional and happy, there's no real need to keep use strict turned on. It is a smidgen bit slower to have 'strict' on, so I keep it off. It can, of course, be turned back on by the individual user. The 'no strict refs' is needed in case someone *does* use strict. If it weren't there, strict would bitch and that's not a good thing, IMHO.


Also, please do not put anything on the CPAN which has your email address as a default value. You should not make people set (or unset) default values within the module, ie:... These things should be done ONLY by passing a hash (I would use a hash) when use'ing the module (or in a new() method, which you don't need to have), or with methods (like Carp::Notify::set_smtp('smtp.me.com'), or Carp::Notify::set_defaults({smtp => 'me.smtp.com',email => 'foo@me.com'}), etc...). Please change this.

These values can be set via individual function calls (within explode or within notify), or when the module is use'd. All done via a hash.

use Carp::Notify (smtp => "smtp.me.com", email => "foo@me.com");

explode(smtp => "smtp.me.com");

So the end user can do it however they'd like. Consequently, it's not required to set the defaults. However, most of the time someone will be using the same smtp server, same notification address, same subject, etc. for this module despite which script it is used in. It's nice to be able to set up defaults in one place that can be overridden later if so desired, or left in place if not so desired.

I like to flag that they should be set in the docs because I really do think that they should so as to avoid headaches later. Best to draw attention to it now than have a difficult-to-debug error later when someone tries to figure out why they can't talk to 'your.smtp.com'

I know that it's port 25 because I was lazy in it. *shrug* I s'pose I flip it back to be a user-configurable var in a future release.

I don't particularly like using POSIX (or any other module, if possible) in a CPAN distribution. It really irks me if a module doesn't work out of the box. Nothing against any other modules, just my personal philosophy on keeping my modules self-contained.

I didn't finish error since I couldn't think of a clever way to do so. It probably doesn't make sense to log that an error occurred to a log file, since one of the ways that error is called is if logging to a file fails. Same argument for having error() email someone with a problem.

I'll think about adding in the ability to pass function refs, but I'm hesitant because I'm not convinced the end user will fully understand what they're used for. If they hand in a ref that just logs to a file or emails, it probably won't do much good. *shrug*