this script normaly deplace files in a directory !

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

#! /usr/bin/perl -W
# this program copy all files in a directory
# and deplace them in a new directory if he doesn't exist
# the program creat him...
use strict ;

my($targetDirFull,$targetDir,
   $origDirFull,$origDir,$bit,
   @allThings,$i,$file,@fileIn);
   
$targetDir = 'multimedia';
$targetDirFull = 'C:\target\' ;
$origDirFull = 'C:\Program Files\origin\' ;
#-------------------------------------
#verification that "Target" exist-
#-------------------------------------
unless( chdir('C:\\') ) { print "Error 1..\n" ;}
else { chdir('C:\\'); }
unless (-e $targetDir) {
       print "creating directory \"Target\"...\n";
       mkdir('Target',0777);
}
#################################################################
## openning "origine";take each file and write them in "target" #
#################################################################
opendir(ORIG,$origDirFull) or die("the directory is unavaible !\n") ;
unless(readdir(ORIG)) {
   print "Error 2" ;
}
else {
     print "open $origDirFull ... [OK]\n";
     @allThings = readdir(ORIG);
     if ($allThings[0] eq "") {
         print "the directory is empty ..normal ?\n";
     }
     else {
           print "there's some file in this directory..\nI deplace the
+m\n";
           while($allThings[0] ne "") {
                 $file = pop(@allThings) ;
          open(INPUT,$file) or die ("Error 3\n");
          @fileIn = <INPUT>;
          close(INPUT);
          
          opendir(TARGET,$targetDir."\"") or die ("this computer is an
+ormaly create..\n");
          open(OUTPUT,">".$file) or die ("Error 4\n");
          foreach $bit (@fileIn) {
              print OUTPUT $bit ;
              }
            close(OUTPUT);
            closedir(TARGET);
      }
      }
}
[download]

In fact i've no idea to cut file but i've just tried ! if someone has some ideas..

that many ways to do it (sic !)

Comment on this script normaly deplace files in a directory ! Download Code

Replies are listed 'Best First'.
Re: this script normaly deplace files in a directory ! by sgifford (Prior) on Apr 10, 2004 at 23:01 UTC
That looks like a perfectly respectable script. You could make it a bit shorter, but for the most part there's really no reason to. One thing I would consider is using the File::Copy module to copy the file; it will be shorter, and perhaps faster on some platforms. Even if you don't use that, reading the entire file into memory then writing the entire thing out wastes some memory. A normal idiom for that is something like: `open(INPUT,$file) or die ("Error opening '$file' for input: $!\n"); open(OUTPUT,">".$file) or die ("Error opening '$file' for output: $!\n +"); while(read(INPUT,$buf,8192)) { print OUTPUT $buf; } close(INPUT) or die "Couldn't close '$file' after input: $!\n"; close(OUTPUT) or die "Couldn't close '$file' after output: $!\n";` [download] One other thing to think about is error handling. It looks like when you encounter an error, you print out a message, and put the entire rest of the program inside an `else` block. If you just print the error and immediately exit, such as with `die`, the flow of your program will be more clear. Also, you might as well use more useful error messages than `"Error 3"`, and print the reason for the error with the `$!` variable. When you call `readdir` the first time, you may be throwing away the first item from the directory; be careful of that. Also, if you want to see if there's something in the directory, using `$allThings[0] eq ""` is a sloppy way to do it; `$allThings[0]` is undefined, and it so happens that when compared to a string it will be converted to an empty string. It's probably best to just see how many elements are in the array, with `@allThings == 0` or `!@allthings`.	[reply] [d/l] [select]
Re: this script normaly deplace files in a directory ! by graff (Chancellor) on Apr 10, 2004 at 23:51 UTC
There is a lot of stuff wrong with the code you've posted. The fact that it won't compile is the least of it. First off, don't use backslashes in file paths -- Perl on Windows knows how to use forward slashes instead and do the right thing, and there are modules (look at File::Spec for starters) that will help you to manipulate file paths without worrying about the problems that backslashes can cause. In your case, note that in this string, at line 12 of the code: `$targetDirFull = 'C:\target\';` [download] the second backslash is being interpreted as an escape for the final single-quote character, which means that there really is no close-quote on this line, and this is why it won't compile. (You did remember to use double backslashes elsewhere, at lines 16 and 17, but forward slashes for file paths is still a better way to go. This bit is strange: `unless( chdir('C:\\') ) { print "Error 1..\n" ;} else { chdir('C:\\'); } unless (-e $targetDir) { print "creating directory \"Target\"...\n"; mkdir('Target',0777); }` [download] This says that if you are able to do "chdir('C:/')", you do it a second time. Then if there is no file or directory named "multimedia" (this is the value assigned to $targetDir), you create a directory called "Target". (Huh?) You don't check whether the mkdir succeeds. In this first use of readdir, you don't keep whatever file name was returned: `unless(readdir(ORIG)) { print "Error 2"; ## not a very informative report! } else { ...` [download] When you read the rest of the directory entries into @allThings and loop through them, you don't check to see which ones are plain files, and which are subdirectories. (Trying to open and the diamond read operator on directories won't work.) Then for each file you try to process, you do another opendir on the output directory, which is quite unnecessary -- you don't even need to do that once. In addition to File::Spec, I suggest you look up File::Copy as well -- it will help simplify what you are doing. Also try using the "-f" or "-d" file checks (these are like the "-e" check that you are already using) as you go through the entries in the input directory, and only deal with things that are data files. Other advice (mostly stylistic): declare and initialize variables where they are needed, rather than putting a bunch of "my" declarations at the top; use "if" more than "unless"; see if you can use spaces instead of tabs when indenting lines (at least when you post code here), so the indents will appear as intended.	[reply] [d/l] [select]