Beefy Boxes and Bandwidth Generously Provided by pair Networks
We don't bite newbies here... much
 
PerlMonks  

Re: Finding old copies of files

by hippo (Bishop)
on Feb 24, 2021 at 11:22 UTC ( [id://11128735]=note: print w/replies, xml ) Need Help??


in reply to Finding old copies of files

Good to hear that you are finding Perl to be useful in solving problems such as this. In the most part your code looks in good shape too.

The one obvious thing I notice when reading through this script is that none of your subroutines take any arguments and none of them return anything (not explicitly at least and they are being called in void context anyway). It isn't critical in a script of this size but since you have it working it might be opportune to try to pass some arguments here and there and see how it goes.

For example you have these:

build_exclude_list() if $exclude_list_file; build_exclude_dir() if $exclude_dir_file;

And each of those subs works on the filename declared outside their scope. If I were writing this, I would pass the filename as an argument and have the subs return immediately if the arg is undef. eg:

sub build_exclude_list { my $file = shift; return unless defined $file; open my $exclude_files, '<', $file or die "Can't open $file: $!"; # ... } # ... build_exclude_list ($exclude_list_file);

Then think about returning the list which it builds rather than assigning to a global hash. Here is how you might return a ref to the hash.

sub build_exclude_list { # ... return \%exclude_list; } my $exclude_list = build_exclude_list ($exclude_list_file);

This makes your subroutine independent from variables declared outside it which in general terms is A Good Thing. It allows for code re-use: you could make the sub perfectly general, put it in a module and use it from multiple scripts without duplicating the code.

Anyway, just something to consider.


🦛

Replies are listed 'Best First'.
Re^2: Finding old copies of files
by Leitz (Scribe) on Feb 26, 2021 at 13:48 UTC
    hippo, thanks! I had some time this morning, and started improving the code based on your comment. While there's still a lot of work to do, I added a method:
    sub write_list_to_logfile { my ( $list, $logfile ) = @_; open my $file, '>', $logfile or die "Can't open $logfile: $!"; foreach my $line ( @$list ){ print $file "$line\n"; } close $file; }
    And then changed the logging method to use the new method:
    if ( keys(%known_dirs) ) { my @known_dirs = keys(%known_dirs); my $known_dirs_filename = "$dir/known_dirs.list"; write_list_to_logfile(\@known_dirs, $known_dirs_filename); }
    The new method works, but I ran out of time before I could streamline it more. I'm thinking I can eliminate the two assignments, but need to test the idea.

    Chronicler: The Domici War (domiciwar.net)

    General Ne'er-do-well (github.com/LeamHall)

Re^2: Finding old copies of files
by Leitz (Scribe) on Mar 04, 2021 at 00:12 UTC

    I've been updating the code a little at a time, so that it takes references as parameters to subroutines. Just used it to delete 4,423 files; hopefully the right ones!

    Chronicler: The Domici War (domiciwar.net)

    General Ne'er-do-well (github.com/LeamHall)

Log In?
Username:
Password:

What's my password?
Create A New User
Domain Nodelet?
Node Status?
node history
Node Type: note [id://11128735]
help
Chatterbox?
and the web crawler heard nothing...

How do I use this?Last hourOther CB clients
Other Users?
Others perusing the Monastery: (7)
As of 2024-04-23 11:51 GMT
Sections?
Information?
Find Nodes?
Leftovers?
    Voting Booth?

    No recent polls found