agoth has asked for the wisdom of the Perl Monks concerning the following question:

I'm afraid my Perl skills are a touch on the rusty side :(, but I was attempting to get a file listing of only the current directory in the same order as ls -tr, whilst still having hold of the filename in a $var.
The method I've ended up with is clunky to say the least, any improvements? 

opendir(DIRH, '.') or die "cant open $!";
my @files = grep !/^\.\.?/ && /.*pm$/, readdir DIRH;
closedir DIRH;

for (@files) {
    $file{$_} = (stat($_))[10];
}

for (reverse sort { $file{$a} cmp $file{$b} } keys %file) {
    print "value : $_ , $file{$_} \n";
}

[download]

Replies are listed 'Best First'.
Re: Directory Listing
by turnstep (Parson) on Jan 09, 2001 at 17:20 UTC
    I'd suggest two small changes. First, don't stat just your $_ variable - since you are doing an opendir, only the names of the files are returned, not the full path. In this particular case, it is okay, but something to get in the habit of so it does not nail you later. Second, you can simply swap the order of the sort arguments instead of using a reverse. Not a big deal, but a little more compact and readable (IMO). I kept the nice map idea from agoth and also added another argument, in case you feel like sorting by somthing else.

    my $dir   = shift || ".";
my $field = shift || 10;
opendir(DIRH, $dir) or die "Cannot open $dir: $!";
my %file = map  { $_ => (stat("$dir/$_"))[$field]}
            grep { !/^\.\.?$/ && /pm$/ }
            readdir DIRH;
closedir(DIRH);

for (sort { $file{$b} cmp $file{$a} } keys %file) {
  print "$_ , $file{$_} \n";
}

    [download]
[reply]
[d/l]
      If your trying to cut it down then you don't really need the hash, since an array dereference is maybe/probably faster than a hash lookup. 
      my $dir   = shift || ".";
my $field = shift || 10;
opendir(DIRH, $dir) or die "Cannot open $dir: $!";
for (
    sort { $b->[1], $a->[1] }
    map  { [ $_, (stat("$dir/$_"))[$field] ] }
    grep { !/^\.\.?$/ && /pm$/ }
    readdir(DIRH)
    ) {
        print join("\t", @$_) . "\n";
}
closedir(DIRH);

      [download]

      Update: fixed a few errors, which turnstep pointed out.
[reply]
[d/l]
Re: Directory Listing
by ChOas (Curate) on Jan 09, 2001 at 17:15 UTC
    For large directories I'd go for a Schwartzian transform
    for the sort (named after Him, not by Him)

    else

    #!/usr/local/bin/perl -w

use strict;

my %File=map {$_,(stat)[10]} </home/choas/*>;
print "$_: $File{$_}\n" for sort { $File{$b} cmp $File{$a} } keys %Fil
+e;

    [download]

    GreetZ!,
      ChOas


    print "profeth still\n" if /bird|devil/;
[reply]
[d/l]
Re: Directory Listing
by agoth (Chaplain) on Jan 09, 2001 at 16:42 UTC
    OK so this is marginally better: 
    opendir(DIRH, '.') or die "cant open $!";
my %file = map { $_ => (stat($_))[10]}
           grep !/^\.\.?/ && /.*pm$/,
           readdir DIRH;
closedir DIRH;

for (reverse sort { $file{$a} cmp $file{$b} } keys %file) {
    print "value : $_ , $file{$_} \n";
}

    [download]
[reply]
[d/l]
      There is almost never a need to use a map and a grep one after the other; they can be combined into a single loop. my %file = map { /^[^.].*\.pm$/ ? ($_ => (stat $_)[10]) : () } readdir DIRH; For each item, if the regex matches, return a two element list (that's the map part), otherwise return an empty list (that's the grep part).

      I combined the original poster's two regexes as well, and made the match a little more explicit.

[reply]
[d/l]
Re: Directory Listing
by runrig (Abbot) on Jan 09, 2001 at 21:11 UTC
    Something no one else has seemed to notice yet on this line:
    my @files = grep !/^\.\.?/ && /.*pm$/, readdir DIRH;

    [download]
    Why exclude the '.' and '..' directories, when you're only including '.pm' files anyway?

    Update: There is also a Death to Dot Star! issue here, so in summation it should be:
    my @files = grep /\.pm$/, readdir DIRH;

    [download]
[reply]
[d/l]
[select]
      my @files = grep !/^\.\.?/ && /.*pm$/, readdir DIRH;

      [download]
      That not only exludes dot and dot-dot, but anything beginning with dot and dot-dot. Maybe it was intended to exclude ".random.pm", perhaps?

      But yes. On a code review, I'd mark that line as "either do what you mean, or write what you mean as a comment".

      -- Randal L. Schwartz, Perl hacker

[reply]
[d/l]

        Update: Read these out of order, but was basically reinforcing the point that all we need is:

        my @files = grep /pm$/, readdir DIRH

        [download]
[reply]
[d/l]
      Just not backtracking and cleaning up my code very well,
      I initially thought just to exclude '.' and '..' but then thought I'd better be a bit more specific with *.pm, and seeing I know what the filename will start with the !/^\./ is pretty superfluous.
[reply]

Back to Seekers of Perl Wisdom