Also you open and completely copy OLD to NEW each time through the loop and rename the files. Might it be better to read CHGFILE once storing the replacement values in an array of arrays and then open OLD exactly one time and for each line loop through the replacements before printing it?

Thanks for your reply. I figured a way to do it differently. The way I decided was to search a complete directory and change any instance of the needed change in all files while creating a backup of the old file. Here is what I did if you can see where this can be improved let mw know:

Here is what I have come up with. I did not actually write out the files and have left those 6 lines commented out as I have a different environment than you and it was not really the point of the exercise. You will also want to throw out my strange if (0) where I set my directory names without being forced to comment yours out.

#These are protected files that are not to be manipulated each name will be #a key in the hash so we can see if the key exists. The value is irrelevant. my %protected = map { $_ => 1 } qw (. .. ipchanger.pl);

By declaring 'my @tmp' inside the loop, we get a new one each time. And then we can push a pointer to the new small array of two elements to the end of the big array changelist. Thus we get our array of arrays and never have to use subscripts. $i was kept only for the print statement. And I chomped before the split to avoid the two regular expressions, although only one should have been needed as the part before the equals should not have had one.

Rather than a long list of file names with 'eq' and '&&' we can use a hash where the keys are the protected filenames. All we have to do is check to see if the filename exists as a key in the hash to know that it is special case.

The for loop is often better replaced with a foreach in Perl. In this case we do not need to keep $i around and can deal with the whole small array at once. $pair is a pointer the the inner array and we can use it to fill $was and $is. Remember that as $pair is a scalar that points to an array and not an actual array you will need to dereference its parts with the '->' notation.

I also added the $ct variable to capture how many times a substitution was made and the print statement will show that fact. Actually the print statement will only print if a subsitution was made to reduce clutter in the output stream.

Code

opendir(DIR, $sourcedir) || die "Can't open superuser path, $!";

When reading a file or directory and the whole filename is already in a variable it is much better to not wrap the single variable in double quotes. Adding the quotes forces perl to create a new string that is exactly the same as the variable that was already known. This does not apply to writing of course as the writing will need to add a '>' to the front of the filename.

Also you should always include $! in your die statements to learn a little more about the error. And you should not end your error message with a new line because you can get $! to tell you more if you do not end with "\n".

Code

if (-d "$sourcedir/$aname") {

Remember that the output from readdir is the filename without any directory information. To use the filename when you are dealing with a directory other than where you currently are you usually have to use it as $dir/$filename.

One other thing to consider, especially as the number or size of your files increases, is that it might be better to read from $sourcedir and write your new files to $outputdir rather than read from $sourcedir, write an exact copy in $backupdir and then a modified copy in $sourcedir. It is probably much easier to move the directories around after the change.