Is there a more concise way to write this in Perl?

This subroutine is passed an array of file handles to close, which it closes one by one using the foreach loop:

sub closef
{
    foreach(@_) {
        my $fh = shift;
        close $fh;
    }   
}

Where all can this simple subroutine be modified to make it bette开发者_运维知识库r?

Should I use close shift instead of doing it in two lines?

The most concise way is to use lexical filehandles which automatically close when they go out of scope:

sub firstline {
    open( my $in, shift ) && return scalar <$in>;
    # no close() required
}

See perldoc perlopentut.

sub closef { close $_ for @_ }

But the close function does have a return value and you would usually do well to examine it. Therefore,

sub closef { map { close $_ } @_ }

foreach(@_) {
    close $_;
}   

or

foreach my $fh (@_) {
    close $fh;
}   

or

close $_ foreach (@_);

Your code isn't doing what you think. You say you want to close each filehandle passed to the routine, but you only close some of them. Consider the following example:

sub t{
        foreach(@_){
                my $tmp = shift;
                print $tmp;
        }
}

t(('a'...'d'));

This will output

ab

Despite the argument list being a, b, c and d. This is because on each iteration shift is being called. To get the kind of behavior you want use

sub t{
        foreach(@_){
                my $tmp = $_;
                print $tmp;
        }
}

t(('a'...'d'));

Or, better, don't make a copy of $_, just alias it:

sub t{
        foreach my $tmp (@_){
                print $tmp;
        }
}

t(('a'...'d'));

There's an issue with the subroutine originally posted.

The shift function expects an array as an argument. The implicit variable inside the foreach is a scalar.

This means that $fh remains uninitialized. The reason why it probably goes unnoticed is because (as eugene y points out) lexical filehandles close themselves when they go out of scope.

Incidentally, use warnings; will notify about such things.

A more idiomatic approach would be to use a while loop:

sub closef {

    while ( my $fh = shift @_ ) {
        close $fh;
    }
}

For more compact notation, a for loop is the way to go:

sub closef { close for @_; }

The equivalent with a while loop is not so readable:

sub closef { close while $_ = shift }

Update

Michael Carman's comment below holds true here. The shift modifies the array being iterated over, which makes it an unsafe operation. This is, in fact, the problem.

Or you can also use map:

map { close $_ } @_;

With map you can even get an array of all results of close calls if you want to process it.

my @results = map { close $_ } @_;