It's a good idea to separate the output from the logic if you can, especially if that output is HTML. Consider this:
use Modern::Perl;
use Email::Send::SMTP::Gmail;
use POSIX qw(strftime);
use File::Find::Rule;
use Template;
my $subject = "ICS -- Files in the Error Folder";
my $email = 'bflieck@xxx.com';
my @bad_files = File::Find::Rule
->file()
->relative
->maxdepth( 1 )
->name( '*.jpg', '*.tif', '*.tiff', '*.psd' )
->in( '/Volumes/photorepos/Partners/WorkHorse/ERROR/PhotographyDro
+p_ERROR' );
# Group files per user
my %bad_files_per_user;
foreach my $file ( @bad_files )
{
my $user = (split /-/, $file)[0];
$bad_files_per_user{ $user } = $file;
}
# Prepare the email text
my $body = Template->new->process( \*DATA, {
subject => $subject,
files => \%bad_files_per_user,
date => strftime( "%m-%d-%y", localtime ),
time => strftime( "%I:%M:%S", localtime ),
});
# Prepare sender
my ( $sender, $error ) = Email::Send::SMTP::Gmail->new
(
-subject => $subject,
-to => $email,
-from => $email,
-body => $body,
# Better to put these in some kind of configuration file
# -debug => 1,
-smtp => 'smtp.gmail.com',
-login => 'kopautomation1@xxx.com',
-pass => 'xxx',
-layer => 'ssl',
-port => '465',
-timeout => 1000
);
die "session error: $error"
unless ref $sender;
# Off we go
$sender->send;
$sender->bye;
__DATA__
<h1>[% subject %]</h1>
<p class="date">[% date %] - [% time %]</p>
<p>Please fix and redrop or delete the following errors</p>
<div class="bad_format">
<h2>Files with bad format</h2>
[% FOREACH user IN files.keys %]
<h3>User: [% user %] ([% files.$user.size %] files)</h3>
<ul class="files">
[% FOREACH file IN files.$user %]
<li>[% file %]</li>
[% END %]
</ul>
[% END %]
</div>
I am using the Template Toolkit and a template stored in the __DATA__ section of the script to generate the mail. Because the output is now separated from the code, I (you) can make the email arbitrarily complex (like adding images or styles or whatnot) without having to change or clutter the the code with print statement.
Also, I am sure you know this already, whitespace is <s>cheap</s> free!
holli
You can lead your users to water, but alas, you cannot drown them.
| [reply]
[d/l] |
…changing directories it does get very confusing, but sometimes a necessary evil…
I am open to being shown incorrect but I don’t think it’s ever necessary and in every case I have come across it so far—including some of my own older stuff—I find it to be code smell.
| [reply] |
I equivocated in my post to the OP.
I also don't see any "necessary evil"
Some Perl libraries like File::Find do chdir's for their work and error recovery from a something like that can be very problematic because you can wind up in some random directory. The &wanted routine should be as simple as possible to avoid this problem.
In general, chdir is a bad idea.
I could think of situations where a chdir to a program's data could simplify the code slightly, but I also don't see "necessary evil", meaning "required".
| [reply] |
Yes, chdir() does have its place.
"advice for the best way to catch the output of the print commands fo storage to become part of the body of the email?
You could use sprintf() which makes a string?
I am not sure whether these user names like "Maur" are actual users on your unix machine? If so, I would use simple SendMail to that user ID and leave it to the users to have a .forward file in their account that forwards to gmail. Don't create a mapping file of some userIDname to gmail address unless for some reason you have to. This adds an extra layer of admin hassle for you. | [reply] |