5
\$\begingroup\$

I often find myself working with files I get from other users that mostly work on Windows machines (I, myself, am on a macOS) and they tend to name their files in all sorts of (weird & difficult to handle) ways.

For example, the files I get may contain any or all of the following:

  • a space
  • a dash
  • a comma
  • one or more dots, not counting the extension dot.

So, I might get these files:

  • This is a file example, 2nd edition.pdf
  • Another.File.Name.I.Get.To.Work.With.pdf
  • this-file-is-actually-named-this-way, 7th edition.pdf

Having said the above, to make my life easier, I've learned some Perl (heard it's great at string handling) and wrote myself a program that takes a directory as an argument and renames all files within it.

By default, I use the _ as the replace_character and .pdf as the file extension, as these are the types of files I mostly work with.

Can anyone of you, kind Perl mongers, review the code in terms of readability, ease of maintenance, and usability?

I am aware of TIMTOWTDI (There’s more than one way to do it!), but, as I've mentioned, I've just started learning Perl and would like to get a (sort of) canonical answer that focuses on writing clean, maintainable Perl, not a throw-away program.

I'm on this Perl:

This is perl 5, version 36, subversion 1 (v5.36.1) built for darwin-thread-multi-2level

And here's my code:

#!/usr/bin/perl

use strict;
use warnings;
use Getopt::Long;
use File::Find;
use File::Copy;
use version; our $VERSION = qv('1.0.0');

=begin comment
Function sanitize_file_name sanitizes the file name by removing spaces, commas, dashes and dots
except the dot before the file extension.
=end comment

=cut
sub sanitize_file_name {
    my ($file_name, $replace_char) = @_;

    # Remove spaces
    $file_name =~ s/\s+/$replace_char/g;

    # Remove commas and dashes
    $file_name =~ s/[,-]+/$replace_char/g;

    # Replace consecutive underscores with a single underscore
    $file_name =~ s/(?<!\.)_+/$replace_char/g;

    # Replace dots with underscores, except the one before the file extension
    $file_name =~ s/(?<=\b)(?<!\.)\.(?![^.]+$)/$replace_char/g;

    return $file_name;
}


# Function to get the file name from a path
sub get_file_name {
    my ($file) = @_;
    my @path = split /\//, $file;
    return $path[-1];
}

# Function to rename the file
sub rename_file {
    my ($old_name, $new_name) = @_;
    my $old_file_name = get_file_name($old_name);
    my $new_file_name = get_file_name($new_name);

    if ($old_file_name ne $new_file_name) {
        rename $old_name, $new_name;
        print "File renamed: '$old_file_name' -> '$new_file_name'\n";
    }
}

# Function to process files in a directory
sub process_files {
    my ($file, $extension, $replace_char) = @_;

    return unless -f $file;
    return unless $file =~ /\.(?:$extension)$/i;

    my $new_file_name = sanitize_file_name($file, $replace_char);
    my $new_full_path = $file;
    $new_full_path =~ s/([^\/]+)$/$new_file_name/;
    rename_file($file, $new_full_path);
}

my $replace_char = '_'; # Default replacement character
my $extension = 'pdf';  # Default file extension
my $help;

# Parse command line options
GetOptions(
    'replace=s' => \$replace_char,
'ext=s' => \$extension,
    'help' => \$help
) or die 'Error in command line arguments\n';

# Display help message
if (defined $help) {
    print "Usage: $0 [options] DIRECTORY\n" or croak $!;
    print "Options:\n";
    print "  --replace=CHAR     Replace spaces with specified CHAR (default: underscore)\n";
    print "  --ext=EXTENSION    Process files with specified EXTENSION (default: pdf)\n";
    print "  --help             Display this help message\n";
    exit;
}

# Check if a directory is provided
if (@ARGV != 1) {
    print "Error: No directory provided\n";
    die "Usage: $0 [options] DIRECTORY\n";
}

my $directory = $ARGV[0];

# Check if the directory exists
if (-d $directory) {
    find(sub { process_files($File::Find::name, $extension, $replace_char) }, $directory);
} else {
    die "Error: Directory '$directory' not found\n";
}
\$\endgroup\$
1

1 Answer 1

6
\$\begingroup\$

You have done an excellent job of using good coding style: good layout, good use of subroutines and good leveraging of other's code (modules). It is great that you used strict and warnings.

Here are some improvements you can make.

Remove the unused File::Copy module.

There is no need to call croak in this line:

print "Usage: $0 [options] DIRECTORY\n" or croak $!;

The print will likely never fail, and even if it did, you want to execute the remaining print lines. Also, croak is not a built-in function; it is from the Carp module, which you did not use.

Consider using the Pod::Usage module and POD code instead of the help statements you have. This gives you a Unix-style manpage for free when you use the perldoc command. For example, you can add this POD at the end of your code:

=head1 SYNOPSIS

renamer [options] DIRECTORY

    Options:
    --replace=CHAR     Replace spaces with specified CHAR (default: underscore)
    --ext=EXTENSION    Process files with specified EXTENSION (default: pdf)
    --help             Display this help message

=head1 DESCRIPTION

A file renamer that gets rid of spaces, commas, dots, and dashes.

=cut

Then you can run this command on your command line to get the manpage (assuming you named the Perl file renamer):

perldoc renamer

I ran podchecker on your code, and it complained about your =begin POD. I added line breaks to fix the errors:

=begin comment

Function sanitize_file_name sanitizes the file name by removing spaces, commas, dashes and dots
except the dot before the file extension.

=end comment

=cut

When I use Getotp::Long, I prefer to use a single hash variable (like %opt) instead of several scalar variables. I find it cleaner.

Indentation is excellent, except for the ext line in the GetOptions call. This is a bit more consistent:

GetOptions(
    'replace=s' => \$replace_char,
    'ext=s' => \$extension,
    'help' => \$help
) or die "Error in command line arguments\n";

I also changed the single quotes to double quotes in the die statement because it prints a literal \n instead of the intended newline character. I ran your code through perlcritic to find that one; sometimes it does find bugs in your code.

I prefer to explicitly declare all functions that are imported from modules:

use Getopt::Long qw(GetOptions);
use File::Find   qw(find);

This makes it clear where the functions are from, and it can limit adding lots of functions into the namespace if a module exports many functions by default.

I prefer to have one statement per line:

use version;
our $VERSION = qv('1.0.0');

Consider the File::Spec module instead of your get_file_name sub.

\$\endgroup\$
1

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.