|
Re: Copying files between directories. How can I improve this code.
by GrandFather (Saint) on Apr 21, 2006 at 23:10 UTC
|
First off, uncomment use strict;! Then add the my to declare each variable. Note though that you need to declare @Filenames outside the loop.
Note that #recursively search $path for files is a lie. There is no recursion and the search will only find files in the top level directory. If you want to actually perform a recursive search you need to do a little more work. At that point File::Find is likely what you want. Taking that route, something like the following (untested) code is what you want:
#!/usr/local/bin/perl -W # force taint checks, and print warnings
use strict; # install all three strictures
use File::Copy;
use File::Find;
my $path = "sourceDirectory"; #define full path to source folder
my $newpath = "destinationDirectory"; #define path to new folder
my @Filenames;
$|++; # force auto flush of output buffer
find (\&doCopy, $path); #Recursively search $path for files to copy
sub doCopy {
return if ! -f $File::Find::name;
chdir($path) || die "cannot move to $path";
my $destFile = $newpath . $_;
print "\$destFile = $destFile\n";
copy( "$_" , "$destFile" ) or print "File $_ cannot be copied. $!\
+n";
}
Note that you will probably have to clean up issues regarding paths to destination sub-directories in copy. You will probably need to do some magic with $File::Find::dir to strip off the root path and append the remaining path to $newpath.
DWIM is Perl's answer to Gödel
| [reply]
[d/l]
[select] |
|
|
Thanks, this is what I've got so far, At the moment all the source files are found, which is brilliant but they dumped into the destination directory and I'm losing the directory structure.
Im going to change the code to append the destination varible with the current directory when copy() fails. Im going to try the reg ex in the morning.Something like this
$\/* to match the / at the end of the string. Then happy days.
#!/usr/local/bin/perl -W # force taint checks, and print warnings
use strict; # install all three strictures
use File::Copy;
use File::Find;
my $path = ""; #define full path to source folder
my $newpath = ""; #define path to new folder
my @Filenames;
#recursively search $path for files
opendir (ORIG , $path);
@Filenames = readdir ORIG;
closedir ORIG;
$|++; # force output of buffer
find (\&doCopy, $path); #Recursively search $path for files to copy
sub doCopy {
return if ! -f $File::Find::name;
chdir($path) || die "cannot move to $path";
my $destFile = $newpath . $_;
#print "\$destFile = $destFile\n";
copy( "$_" , "$destFile" ) or $path = $File::Find::dir ;
#print "$_ cannot be copied $!. $File::Find::dir \n" ;
}
| [reply]
[d/l] |
|
|
#!/usr/local/bin/perl -W # force taint checks, and print warnings
use strict; # install all three strictures
use File::Copy;
use File::Find;
my $path = "C:/Perl/eg"; #define full path to source folder
my $newpath = "c:/Delme"; #define path to new folder
$| = 1; # force auto flush of output buffer
chdir($path) || die "cannot move to $path";
find (\&doCopy, $path); #Recursively search $path for files to copy
sub doCopy {
return if ! -f $File::Find::name;
my $subTree = substr $File::Find::name, 1 + length $path;
my $destFile = "$newpath/$subTree";
print "\$destFile = $destFile\n";
#copy ($_, $destFile) or print "File $_ cannot be copied. $!\n";
}
Prints:
$destFile = c:/Delme/example.pl
$destFile = c:/Delme/Readme.txt
$destFile = c:/Delme/aspSamples/ado1.asp
$destFile = c:/Delme/aspSamples/ado10.asp
$destFile = c:/Delme/aspSamples/ado11.asp
$destFile = c:/Delme/aspSamples/ado12.asp
$destFile = c:/Delme/aspSamples/ado13.asp
$destFile = c:/Delme/aspSamples/ado14.asp
$destFile = c:/Delme/aspSamples/ado15.asp
...
Note that you needn't interpolate variables into strings to pass them as parameters, just pass the variable: copy ($_, $destFile) for example.
DWIM is Perl's answer to Gödel
| [reply]
[d/l]
[select] |
|
|
|
|
Re: Copying files between directories. How can I improve this code.
by Fletch (Bishop) on Apr 21, 2006 at 23:08 UTC
|
Well, for one thing there's your formatting, or rather the total lack thereof; see perlstyle and/or install perltidy.
Another nit is you don't check the return from opendir; ALWAYS CHECK THE RETURN VALUE FROM SYSTEM CALLS.
Then there's the strange line with @Filenames all by itself in void context; we'll be generous and presume that's cruft from trying to print its context that got left in.
I would have written your print line in the last loop as print "\$destFile = $destFile\n"; one backwhack's less noise than the separate concatenation (that's again a minor nit).
As for an actual logical error, it doesn't appear that $destFile will contain a useable path as $newpath doesn't end in a "/". Consider using File::Spec for a portable way of building paths.
Update: Aaah, that @Filenames on its own was probably your trying to declare it; as is pointed out below you need to do that outside the loop.
| [reply]
[d/l]
[select] |
Re: Copying files between directories. How can I improve this code.
by runrig (Abbot) on Apr 21, 2006 at 23:10 UTC
|
Some readability could be gained by some indentation. A run through perltidy wouldn't hurt.
Check the return of opendir. In checking it and chdir, include the $! variable in the error message.
Look into strict and warnings, when you're ready to try that again, it'll mostly involve just putting my in front of the first occurance of every variable, but try to understand the what and why of it.
There is no need to quote your arguments in copy().
Your basic method is sound, though.
| [reply]
[d/l] |
|
|
Is there anything in BBEdit that would do this. Can I link it to perltidy somehow? I'll find out now.
I believe that my gives each varible a local scope. This stops bugs due to ambiguities.Correct me if I wrong please.
Thanks for all your help BTW.
| [reply] |
|
|
#! /usr/bin/perl
# ~/Library/Application Support/BBEdit/
# Unix Support/Unix Filters/perltidy.pl
use strict;
use warnings;
use Perl::Tidy;
my @destination;
Perl::Tidy::perltidy(
source => [ <> ],
destination => \@destination,
);
print @destination;
bound to ctrl-t on my BBEdit. | [reply]
[d/l] |
Perhaps linking might be useful?
by roboticus (Chancellor) on Apr 21, 2006 at 23:47 UTC
|
richill:
I see that other PerlMonks have already suggested improvements to your code and such. So on a different note, I thought I'd suggest that you might want to investigate linking your files instead of copying them.
This would save you the overhead and disk space of copying files with similar results. Obviously, if you're copying the file to make a backup before changing the contents of the source file, linking isn't what you want.
--roboticus | [reply] |
Re: Copying files between directories. How can I improve this code.
by salva (Canon) on Apr 22, 2006 at 09:37 UTC
|
there is a bug that nobody has commented before here ...
while ($name = readdir(ORIG)){
A file named 0 (= number zero, that is unlikely, but not impossible) would cause the loop to end.
You have to check if the value is defined instead:
while (defined($name = readdir(ORIG))){
Though, the best solution is to use readdir in list context replacing the full while loop:
my @Filenames = readdir ORIG;
| [reply]
[d/l]
[select] |
|
|
Modern versions of Perl aready provide this:
$ perl -MO=Deparse -e'while ($name = readdir(ORIG)) {}'
while (defined($name = readdir ORIG)) {
();
}
-e syntax OK
| [reply]
[d/l] |
Re: Copying files between directories. How can I improve this code.
by rhesa (Vicar) on Apr 22, 2006 at 13:10 UTC
|
While this may be a good exercise in directory traversal, i'm wondering what would be wrong with cp -r $src_dir $dest_dir.
If you really want to do it in Perl, you might find File::Find helpful. I noticed that you have a comment saying #recursively search $path for files, but your code doesn't recurse into subdirectories at all :) GrandFather already pointed this out
Another minor detail is that you're not distinguishing between files and directories, and I'm pretty sure that File::Copy's copy function doesn't handle copying directories. | [reply]
[d/l]
[select] |