1) Your || has too high precedence.
open ACCESS, ">> $log" || die "Cannot open $log: $!";
is equivalent to
open ACCESS, (">> $log" || die "Cannot open $log: $!");
which is equivalent to
open ACCESS, ">> $log";
Instead, use
open(ACCESS, ">> $log") || die "Cannot open $log: $!";
or
open ACCESS, ">> $log" or die "Cannot open $log: $!";
2) You don't check if chdir succeeded.
3) The three argument form of open is safer.
4) Using lexicals for filehandles is cleaner.
5) The following is easier to read and less error-prone:
my $log = $file;
$log =~ s#^.*/##;
$log =~ s#\.[^.]$##;
$log .= '.log';
6) join might be a better choice instead of all those string concatenations.
7) Finally, you could eliminate the chdir as follows:
$log = "/perl/web/examscores/$courselocation/access/$log";
Fixed code:
my $clocktime = localtime;
my $log = $file;
$log =~ s#^.*/##;
$log =~ s#\.[^.]$##;
$log = "/perl/web/examscores/$courselocation/access/$log.log";
open(my $access, '>>', $log)
or die("Cannot open $log: $!\n");
print $access (
join(' ',
$idinput,
$lname,
$fname,
$clocktime,
$useripaddress,
$time,
), "\n");
close($access);
By the way, localtime returns a string with spaces in it (in scalar contect).
|