How to improve this code

[anglaissam]

if (defined($ARGV[0]) && $ARGV[0] ne "") {
    $f=""; #initialize $f
    $dir = $ARGV[0];
    chomp($dir);
    #directory paths should use '\' instead of '/'
    if ($dir =~ /^\w:\//) {
        $_ = $dir;
        s/\//\\/ig;
        $dir = $_;
    }
    #seperate filename from directory path if given
    if ($dir =~ /.*\.htm$|\.html$/i) {
        $f = substr($dir, (rindex($dir,"\\"))+1,);
        $dir = substr($dir, 0, (rindex($dir,"\\"))+1);
        chdir("$dir");
    }
}
#if no directory given, default to script directory
else {
    $dir = substr($0, 0, (rindex($0,"\\"))+1);
}
#Directory path must end with '\'
if ($dir !~ /\\$/i) { $dir = $dir . "\\"; }

The above code is what i am currently using to initialize the directory and filename for my script. The program will always be used with a html file, so there is no worries about other file formats being passed through the code.

While the code works, it seems to me to be a little 'clunky' in design. I don't know if its possible, but any suggestions on improving the code to make it cleaner and or faster?

Thanks for any help in advance.

[2teez]

Hi anglaissam,

Foremost, please use 'warnings' and 'strict' in your script, it will have in a million ways; talking from experience.

Secondly, don't program in a cage!!! What do I mean?
use what has been provided in the program. Instead of doing the job all by yourself.
For example in your program, use the subroutines namely dirname and basename from the module File::Basename that comes with your Perl Version.

In fact, you could write this:

use warnings;
use strict;
use File::Basename qw(dirname basename);

# this is used if nothing is given from the CLI
die "The file is not specified" unless defined $ARGV[[0];

## to seperate the directory from the file
my $path_from_cli = $ARGV[0];

my $dir  = dirname($path_from_cli); # get your directory path name
my $file = basename($path_from_cli); # get your filename

Then you can test for anything you want to test for, without any "issues" at all. All your work easily done.
So, you don't have to take on the seperator of the path yourself.
Infact, there are several Module from CPAN that can do that for you, that is why I said don't program in a CAVE!

NOTE: The code above is not tested, but I believe it should work. And if you have any other question please feel free ask.

Thanks.

[anglaissam]

Going through CPAN i chose to use File::Spec. The following is my adjusted code with the exception that i have not yet implimented "Strict". So ignoring the fact that the code isn't strict, is there any other advice on improving the general readability/quality of this code?

Thanks for your previous advice 2teez.

$no_file = 1; #0 means there is a file indicated in path. 1 means there isn't.

if (defined($ARGV[0]) && $ARGV[0] ne '') {
    $path = $ARGV[0];
    if ($path =~ /.*\.html?$/i) { $no_file = 0 }
}
else { $path = substr($0, 0, (rindex($0,"\\"))+1); }

($vol,$dir2,$f2) = File::Spec->splitpath( $path, $no_file );
$dir2 = File::Spec->catpath($vol, $dir2);
$dir2 =~ s/\//\\/ig;
if ($dir2 !~ /\\$/i) { $dir2 = $dir2 . "\\"; }

chdir("$dir2");

if (defined($ARGV[0]) && $f2 ne '') { $thefiles[0] = $f2; } #ARG1 for specific file.
else {
    opendir(DIR, $dir2) || die("Warning1: Cannot open directory $dir2");
    @thefiles = readdir(DIR);
    closedir(DIR);
}