Let me suggest an alternate approach. First get rid of the opendir calls by delegating to File::Find.

  use File::Find;

  find(\&eachfile, "/data/");
[download]

  sub eachfile {
      my $path = $File::Find::name;

      return if -d $path;

      $path =~ s|^.*/(.+)_logs/|| or return; # or log
      my $item = $1;

      if ( $path =~ m|(.+)\.(\d+)$| ) {
          my($company,$quantity) = ($1,$2);
          inventory($company, $item, $quantity);
          return;
      }

      # and so on for each remaining case

      # eventually logging any that you can't handle
  }
[download]

This would seem to me to be a very easy thing for whoever comes along after you to understand and maintain. The most they would need to understand is File::Find, and that's a low hurdle.

Granted, this leaves you with the equivalent of a string if/elseif*/else, but it seems easy enough to follow.

One thing that could extend this approach and possibly eliminate the large quantity of ifs is to templatize the conditional.

Set it up so that the test and actions are defined in a config file and work via a generic test/action method.

Granted the performance will probably drop, but the maintainability should increase.

There also might be modules that support this sort of templating of a function, but i haven't seen them yet. (as i haven't had the need yet)

Alternatively, assuming that I understand the problem right, you can create an array of hashes as shown below:

my @actions = (
   { 
     match => sub { $_[0] =~ /^(\w*)\.(\d*)$/; },
     type  => "inventory",
     action => &add_inventory;
   },
   {
     match => sub { $_[0] =~ /^maimed(\w*)$/; },
     type => "maimed",
     action => &handle_maimed;
   },
   {
     match => sub { $_[0] },
     type => "default",
     action => sub { print "I cannot handle ", $_[0]; }
   }
);

# much later

foreach my $file ( @filelist ) {
   foreach my $action ( @actions ) {
      if ( my @parts = &{ $action->{ match } }( $file ) ) {
          print "Am doing $action->{ type } on $file\n";
          &{ $action->{ action } }( @parts );
          last;
      }
   }
}
[download]

-----------------------------------------------------
Dr. Michael K. Neylon - mneylon-pm@masemware.com || "You've left the lens cap of your mind on again, Pinky" - The Brain
"I can see my house from here!"
It's not what you know, but knowing how to find it if you don't know that's important

Seemingly minor changes to your code can break code that uses Switch. I can't recommend anyone use it in production code.

        - tye (but my friends call me "Tye")

Agreed. It was only ever intended to be a proof-of-concept for Perl 6, and isn't at all ruggedized against the exigencies of the production environment.

However, it will be core in 5.8 and I've been working on toughening it up for that. You may find it is now far more reliable than it used to be.

In addition, the next release will be layered over Filter::Simple, rather than Filter::Util::Call, and that will definitely make it more robust.

I think dws above is on the right track. I am sure things are a little more complicated than you alude to, so use File::Find to suck the whole directory structure into a hash, and then use Parse::RecDescent to step through with a simple grammar and do different stuff depending on what it finds. That way next year when the structure (or database) gets slightly more complicated (it always does!), it just takes a slight grammar change. Good luck.

drinkd

use diagnostics;
use strict;
use Data::Dumper;

my %sprockets;
my %cogs;
open FILES, "c:/cygwin/bin/find.exe data  -type f -print|" or die "no 
+data";
while(<FILES>){
        m[sprocket_logs/(\w+)\.(\d+)]   and $sprockets{$1} = $2;
        m[cog_logs/(\w+)\.(\d+)]        and $cogs{$1} = $2;

}
print Dumper(%sprockets, %cogs);
[download]

HTH, --traveler

A hierarchical data structure is a natural for an XML document. For your case it could look like this:

<data>
  <sprocket_logs>
    <spacely>200</spacely>
  </sprocket_logs>
  <cog_logs>
    <cogswell>100</cogswell>
    <astro>1</astro>
    <maimed_a_robot />
  </cog_logs>
</data>
[download]

Then you have many choices for eliminating your big switch statement. Here are a few ideas that came to mind; there are better ideas, also:

• Access your XML as if it were a database using DBD::AnyData.

• Create a multilevel hash from the XML with XML::Simple or one of the SAX modules. Make a hash of subs that perform each action that you would have in your giant switch statement. Dispatch these subs as you loop through the multilevel hash.
• Use XSLT to replace the perl code that generates the existing HTML view of your directory structure.

It should work perfectly the first time! - toma