How can I open a file only if it is not already open, in Perl?

Question

If I have a subroutine that opens a file what is the best way to ensure it opens it only upon the first time the subrountine is called? I have this but not sure if its best practice:

{
my $count = 0;
sub log_msg {
    my ($msg,$name) = @_;

    if ($count == 0) {
        my $log_file_name = "/tmp/" . $name;
        open my $log_fh,">",$log_file_name or  croak "couldn't open $log_file_name : $!";
        print $log_fh "$timestamp: created and opened $log_file_name\n";
    }
    $count++;
    }
}

Answer 1

Well, for starters, the $count++ should go inside your if statement and can be changed to simply $count=1. You may also want to rename $count to $file_opened_flag or something more meaningful, as well. Other than that, I see nothing wrong with this.

Answer 2

Sounds like a good reason to use a state variable. Store the filehandles in a persistent hash.

#!/usr/bin/perl

use 5.010;
use strict;
use warnings;

sub log_msg {
  state %fh;
  my ($msg, $name) = @_;

  unless ($fh{$name}) {
    warn "Opening $name\n";
    open $fh{$name}, '>', $name or die $!;
    print {$fh{$name}} scalar localtime, " Opened file\n";
  }

  print {$fh{$name}} $msg, "\n";
}

log_msg('Message1', 'first.log');
log_msg('Message2', 'first.log');
log_msg('MessageA', 'second.log');
log_msg('MessageB', 'second.log');

Note the extra set of braces around the filehandles in the print call. That's because print is a bit picky about what you can use as its filehandle argument.

Answer 3

I don't think there is anything wrong with using per se but if there is really only 1 file to track, why not just keep $log_fh in the closure and use if(!$log_fh->opened()) instead of a count variable?

Answer 4

The best way is to use Log::Log4perl so you don't have to think about it and you can focus on your real task.

Aside from that, you can use some of the tricks for files and filehandles that we cover in Effective Perl Programming. Luckily for you, that's also the free chapter that our publisher gives away.

In short, you don't want to think about that in your logging routine. It's cluttercode. Instead, create a method that either returns the cached filehandle or opens it (this is a lot like a method you'd use to ping a database handle and reconnect if needed):

 sub log_msg {
      my( $self, $msg, $name ) = @_;

      print { $self->get_fh_by_name( $name ) } $msg;
      }

 BEGIN { # to define variables before the subroutine
 my %log_fhs;
 sub get_fh_by_name {
     my( $self, $name ) = @_;

     return $log_fhs{$name} if defined $log_fhs{$name};

     open my $log_fh, catdir( $base_dir, $name ) or croak "...";
     print $logfh ...

     $log_fhs{$name} = $log_fh;
     }
 }