tags:

views:

97

answers:

2
sub DirectoryExists {

    my $param = shift;

    # Remove first element of the array
    shift @{$param};

    # Loop through each directory to see if it exists

    foreach my $directory (@{$param}) {

        unless (-e $directory && -d $directory) {
                return 0;
        }
    }

    # True
    return 1;
}

Is there any way to optimize this code?

Is there any good way to optimize this code?

+7  A: 

That algorithm is pretty efficient, because it stops at the first item but you might want to give List::Util::first a try.

use List::Util qw<first>;
#...

return defined first { -e && -d } @$param;

The only major optimization would be that it runs in the C-layer. It's also a pretty recognizable idiom in Perl, and so despite the golf look, the purpose is to "speak perl", not to golf.

List::MoreUtils::any would give you a similar effect and as well, it's a better fit to what you're trying to express: you're asking if any in the array are directories. (a hint though, stack parameter passing is slightly to significantly faster than constructing a reference and passing it--at least in my tests.)

Anyway, here's what it looks like:

return any { -e && -d } @$param;

Means to return true if any satisfy that expression. any often runs in the C-layer, if the module could load its XS version. Otherwise it's "Pure Perl" and probably runs similar to yours.

However, I'm pretty sure you don't have to test for both existence and directory. I'm pretty sure that if the file does not exist, it's not going to be seen as a directory. So, you could collapse it to one condition.

Axeman
Eric Strom
@Eric Strom: I find your optimisation a bit funny actually -- yes it works and will save a kernel filesystem op, but as Axeman and planetp noted, so does this: `{ -d }` :)
j_random_hacker
+1  A: 
Sinan Ünür
@Sinan: Just a guess - perhaps the OP wants to `shift` to get rid of the `.` or `..` entries in a typical directory list.
Zaid
@Zaid: Yes, that's something you may want to do, but what Sinan means is that the `shift` is actually modifying the *caller's* array -- which is probably not something you want to do. Something I'd put a big loud comment next to at least if that's what I intended.
j_random_hacker
Ah, OK, it was the second `shift` I had in mind when I made my earlier comment.
Zaid
@Zaid: Um, I'm talking about the 2nd `shift` too. `$param` is a ref to an array owned by someone caller-side; `shift @{$param};` will modify that array.
j_random_hacker
@j_random_hacker : Agreed, and it's because the first `shift` causes `$param` to refer to the caller-side array. It doesn't create a lexical copy like `@_` would.
Zaid