in reply to Guest Cleanup

my $count = 0; my @mkdir = ( "$year:$mon:$mday", "$hour:$min:$sec"); chdir ("/home/guest-content"); while ($count ne @mkdir){ system "mkdir $mkdir[$count]"; chdir "$mkdir[$count]"; $count ++; }

O.k., you've got some things to learn, so let's start with the above.

First and foremost: don't use ne for comparing numbers. It and its kin — eq, lt, gt, etc. — are for comparing strings only. For numbers, use !=, ==, <, >, etc. (See Equality Operators.)

Second, while the use of $[ is officially discouraged, it's still much safer to relate indices of an array to $#a rather than scalar(@a).

Third, stylistically, you might want to watch out for unnecessary quoting of scalars.

Fourth, don't shell out to execute a command that Perl has built in.

Fifth, you can use the C-style for loop as a syntactic condensation of your $count testing and incrementing.

Putting the above together:

my @mkdir = ( "$year:$mon:$mday", "$hour:$min:$sec"); chdir ("/home/guest-content"); for ( my $count = 0; $count <= $#mkdir; $count++ ) { mkdir $mkdir[$count]; chdir $mkdir[$count]; }

Sixth, unless the array is very long, it's more idiomatic Perl to use iterate the index over a list of the indices using the range operator:

for my $count ( 0 .. $#mkdir ) { mkdir $mkdir[$count]; chdir $mkdir[$count]; }

Seventh, it's bad style to iterate through arrays by index, unless for some reason you need the index for something else (such as iterating through multiple arrays in parallel). You can iterate on the elements of the array directly:

for my $elem ( @mkdir ) { mkdir $elem; chdir $elem; }

Eighth, consider eliminating variables which are only used once. You can almost always operate on a list as well as on an array.

for my $elem ( "$year:$mon:$mday", "$hour:$min:$sec" ) { mkdir $elem; chdir $elem; }

Lastly, check to see if there's a module that already does what you're trying to do.

use File::Path; mkpath( "$year:$mon:$mday/$hour:$min:$sec" );

One more thing: Consider that using ":" (colon) in pathnames is non-portable. It's illegal on Windows, and is the path separator on Mac.

Update: One more thing... You should always check the result of system calls, including the built-in ones.

mkdir "guest" or die "$! - mkdir guest"; system "cp -R default/* guest" and die "cp -R failed";

We're building the house of the future together.

Replies are listed 'Best First'.
Re^2: Guest Cleanup
by ysth (Canon) on Oct 01, 2006 at 17:32 UTC
    $elem is a lousy variable name, just as bad as $count. Also note that mkpath throws exceptions on some errors; when switching to it instead of mkdir, consider whether or not you want to catch them with eval { }.
      $elem is a lousy variable name, just as bad as $count.

      I respectfully disagree. It's bad, but not as bad. $elem carries very little information; $count is actually misleading.

      In such situations, I tend to use $_, which has the advantage of being known, a priori, as a list iterator, by virtue of its special relation to foreach and while(<>).

      for ( "$year:$mon:$mday", "$hour:$min:$sec" ) { mkdir $_ or die "mkdir $_ - $!"; chdir $_ or die "chdir $_ - $!"; }
      We're building the house of the future together.