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

Hello fellow monks. I am not sure if I chose the best title, but its all I could think of. I am wondering if there is a better way to accomplish the task I am working on.

I am working on a script that reads the names of several directories into a hash and then 1) prints the keys to that hash into a benchmark file and 2) prints a report of all the dirs it found. I was later asked to add the ability to read some dir names from a config file and that those names should be skipped in the printed report, but should still be printed into the benchmark file. Here is the solution I came up with. I have stripped the code down to what should be the relevant parts. 

use strict;

my ($config_dirs) = &read_conf();
my ($dirs) = &search_dirs();
&print();

sub read_conf {
       my %config_dirs;
       .... # read the conf file
       return \%config_dirs;
}

sub search_dirs {
       my %dirs;
       .... # search the dirs
       return \%dirs;
}

sub print {
       foreach (keys %$dirs) {
              my $no_print = 0;
              foreach my $skip (keys %$config_dirs) {
                     if (/^$skip/) {
                            $no_print = 1;
                            last;
                     }
              }
       print REPORT "$_\n" unless ($no_print == 1);
       }
}

[download]


Now that works, but it just seems like there has to be a better way to do that. Any suggestions?

If it helps some examples of what would be in the config file would be things like /home and some of the found dirs would be things like /home/bob and /home/ted. Thanks.

-Prime

Replies are listed 'Best First'.
Re: Efficiency Question
by dragonchild (Archbishop) on Mar 21, 2002 at 19:33 UTC
    Why not do something like: 
    foreach (keys %$dirs)
{
    next if $config_dirs->{$skip};
    # Do stuff
}
---- Or ----
foreach (keys %$dirs)
{
    next if grep {/^$skip/} keys %$config_dirs;
    # Do stuff
}

    [download]
    The second is (as far as I can tell) identical to your code. The first assumes that the entries in $config_dirs is string-wise equal to the entries in $dirs. YMMV

    ------
    We are the carpenters and bricklayers of the Information Age.

    Don't go borrowing trouble. For programmers, this means Worry only about what you need to implement.

[reply]
[d/l]
      I'm sorry I forgot to address that. The dirs in %dirs will never exactly match the dirs listed in %config_dirs. The config file will only contain high level directories like /home /etc /usr etc. But the %dirs is going to contain things like /usr/local/bin etc. so an exact match won;t be possible.
[reply]
        If you're brave, you can construct a regex which you can bang your various directories against. This can be done quite simply like so:
        my $skip = join ('|', map {quotemeta} keys %$config_dirs);
        This will look something like "/usr/bin|/home|/var" or what have you. Now you can just go and check against this, like dragonchild suggested, but with a slight mod: 
        foreach (keys %$dirs)
{
    next if /^($skip)/;
    # Do stuff
}

        [download]
        Presumably if the value of $skip does not change within your program, you can use the /o option to only "compile once" your regular expression.
[reply]
[d/l]
[select]
        But then if you have /home in the config file, you should not print /home/foo? (I did not understand that too well) If you are going to have only high level directories (and by that I mean subdirectories in /), and the directories you have to decide wether or not to print are subdirectories of these high level ones, then I guess you could strip everything after the second '/' and then make a direct comparaison... 
        sub print_report {
    foreach (keys %$dirs) {
        print BENCHMARK $_;
        m#^/[^/]+/#;
        next if defined $config_dirs->{$&};
        print "$_\n";
    }
}

        [download]
        (I hope I understood the problem correctly)
[reply]
[d/l]
Re: Efficiency Question
by vladb (Vicar) on Mar 21, 2002 at 20:16 UTC
    To start with, rename print subroutine to something that wouldn't collide with the built in Perl method ;).

    Secondly, why not pass arguments to that print subroutine anyways? It's not a very good idea to keep global variables, especially in code larger than a few pages, which I assume your code is since here you only chose to present a stripped down portion of your entire program right?

    As far as "the better way of doing this" goes, I think I don't have much to add to the first reply ;).



    "There is no system but GNU, and Linux is one of its kernels." -- Confession of Faith
[reply]
[d/l]
[select]

Back to Seekers of Perl Wisdom