Here's the biggest part, my if-elsif-else statements, and looping constructs.

#!/usr/bin/perl
#use strict;
use warnings;
use Carp;
use File::Find;

# globals
$g_P4Base = "//depot/main/src/";


sub BuildPerforceFilePath {
    my ($imagefile, $path) = @_;
    my $ImageFilePath = "";
    
    # strip the src="/ or src=" from the $imagefile variable. 
    if ($imagefile =~ m/src=\"\//i) {
        $imagefile = "\/" . $';
    }
    elsif ($imagefile =~ m/src=\"/i) {
        $imagefile = $';
    }

    # remove double-quotes at end of string
    if ($imagefile =~ m/(\")$/) {
        $imagefile = $`;
    }

    my $fixedimagefile = $imagefile;
    # Append the forward slash at the beginning if there isn't one...
    if (!($fixedimagefile =~ m/^\//)) {
        $imagefile = "\/" . $imagefile;
    }
        
    # check to see if this image file is in the eMedSample path
    if ($path =~ m/([a-zA-Z0-9\_\-\/])+MMS-Apps\/eMedSample/i) {
        $ImageFilePath = "MMS-Apps/eMedSample/WwwRoot" . $imagefile;
    }
    # check to see if this image file is in the eSignature path
    elsif ($path =~ m/([a-zA-Z0-9\_\-\/])+MMS-Apps\/eSignature/i) {
        $ImageFilePath = "MMS-Apps/eSignature/eSignature.WEB" . $image
+file;
    }
    # check to see if this image file is in the Partner-Apps path
    elsif ($path =~ m/([a-zA-Z0-9\_\-\/])+MMS-Apps\/Partner-Apps/i) {
        if ($path =~ m/([a-zA-Z0-9\_\-\/])+MMS-Apps\/Partner-Apps\/Hen
+ryScheinSC/i) {
            $ImageFilePath = "MMS-Apps/Partner-Apps/HenryScheinSC/Henr
+yScheinSC.WEB" . $imagefile;
        }
        elsif ($path =~ m/([a-zA-Z0-9\_\-\/])+MMS-Apps\/Partner-Apps\/
+SampleCenter/i) {
            $ImageFilePath = "MMS-Apps/Partner-Apps/SampleCenter/Sampl
+eCenter.WEB" . $imagefile;
        }
        elsif ($path =~ m/([a-zA-Z0-9\_\-\/])+MMS-Apps\/Partner-Apps\/
+Lilly/i) {
            $ImageFilePath = "MMS-Apps/Partner-Apps/Lilly/LillyConnect
+/LillyConnect.WEB" . $imagefile;
        }
        elsif ($path =~ m/([a-zA-Z0-9\_\-\/])+MMS-Apps\/Partner-Apps\/
+PartnerTest/i) {
            $ImageFilePath = "MMS-Apps/Partner-Apps/PartnerTest/Partne
+rTest.WEB" . $imagefile;
        }
        else {
            $ImageFilePath = "Not sure what the path is here: $path\n"
+;
        }
    }
    else {
            $ImageFilePath = "Not sure what the path is here: $path\n"
+;
    }
    return $ImageFilePath;
}

# This function in addition to 
sub ReportFileFound {
    my ($filename, $imagefile, $linenum, $bBuildFilePath) = @_;
    
    # Convert backslashes to forward slashes
    my $P4ImagePath = $filename;
    $P4ImagePath =~ tr/\\/\//;

    # replace section of path with that of the Perforce Path
    if ($bBuildFilePath == 1) {
        my $ImageFilePath = BuildPerforceFilePath($imagefile, $P4Image
+Path);
        $ImageFilePath = $g_P4Base . $ImageFilePath;
        $ImageFilePath =~ tr/\\/\//;
        InsertIntoList($ImageFilePath);
    }
    else {
        InsertIntoList($filename);
    }
}

# what we are doing now is to ignore all duplicate references to files
+ so that the list that is written only contains the 
sub InsertIntoList {
    my $ImagePath = shift(@_);
    my $aLength = $#FinalImageFileList + 1;
    my $bFound = 0;
    
    if ($aLength > 0) {
        
        # lets see if we can find the string in the array already...
        foreach $tmpImageFile (@FinalImageFileList) {
            
            #its not here so lets insert it...
            if ($tmpImageFile eq $ImagePath) {
                $bFound = 1;
                last;
            }
        }
        if (!$bFound) {
            push @FinalImageFileList, $ImagePath;
            print hLOGFILE "$ImagePath\n";
        }
    }
    else {
        push @FinalImageFileList, $ImagePath;
        print hLOGFILE "$ImagePath\n";
    }
}

sub CheckForImageReferencesInFiles {
    my $filename = $_;
    my $line = "";
    my $linenum = 1;
    
    open hFile, $filename or die "Failed to open $filename\n";
    while ($line = <hFile>) {
        chomp($line);
        
        if ($line =~ m/src="([a-zA-Z0-9\_\-\/])+\.(gif|jp[e]?g|bmp)"/i
+) {
            $line = $&;
            ReportFileFound($saved_name, $line, $linenum, 1);
        }
        $linenum++;
    }
    close hFile;
}

sub CheckForImageReferencesInSQL {
    my($server, $username, $password, $DB) = @_;
    use MSSQL::Sqllib;

    # Log into the server.
    my $oSql = sql_init($server, $username, $password, $DB);
    my $p4Path = $g_P4Base;
    
    # now build the P4 depot path based on the DB we are using
    if ($DB eq "ems_prod") {
        $p4Path .= "MMS-Apps/eMedSample/Wwwroot";
    }
    elsif ($DB eq "esignature") {
        $p4Path .= "MMS-Apps/eSignature/eSignature.WEB";
    }
    elsif ($DB eq "ems_directchannel") {
        
    }
    else {
        
    }
    my $p4FQP = $p4Path;
    # Run a query.
    my @QueryResult = $oSql->sql("SELECT imgPath From Images Where Sta
+tusCode = 1");      # Just print the results, it's a list of hashes.
    foreach $QueryResult (@QueryResult) {
        foreach $row (keys %$QueryResult) {
            $p4FQP .= $$QueryResult{$row};
            ReportFileFound($p4FQP, 0, 0, 0);
        }
        $p4FQP = $p4Path;
    }
}

sub ListFileNames {
    $saved_name = $File::Find::name;

    # looking for filenames whose extensions are the oens I want (jpg,
+ jpeg, js, xml)
    if ($saved_name =~ m/\.(cs|asp[x]?|js|xml)$/i) {
        
        # replace all forward slashes with back-slashes
        $saved_name =~ tr/\//\\/;
        
        # Look for image files beign referenced in the file.
        CheckForImageReferencesInFiles($saved_name);
    }
}

@ARGV = (".") unless @ARGV;
$g_FirstCall = 1;
$g_Searchpath = shift(@ARGV);
$g_logfile = "C:\\Scan4Images.txt";
@FinalImageFileList = ();

if (-e $g_logfile and $g_FirstCall != 0) {
    unlink $g_logfile;
    $g_FirstCall = 0;
}
open hLOGFILE, ">>$g_logfile" or die "Could not open the file $g_logfi
+le\n";


print hLOGFILE "From Source code files:\n\n";
find(\&ListFileNames, $g_Searchpath);
print hLOGFILE "\n\n";
print hLOGFILE "From SQL Queries:\n\n";
CheckForImageReferencesInSQL("emsdbserver", "emsuser", "Mmsmms12", "em
+s_prod");

close hLOGFILE;
[download]

#use strict;
[download]

Ouch. Use strict and warnings!

    if ($imagefile =~ m/src=\"\//i) {
        $imagefile = "\/" . $';
    }
    elsif ($imagefile =~ m/src=\"/i) {
        $imagefile = $';
    }
[download]

If you look at perlvar, under $' and $`, it says "The use of this variable anywhere in a program imposes a considerable performance penalty on all regular expression matches." If your performance isn't a problem, don't worry about it. All I would suggest is you use English and name them $POSTMATCH and $PREMATCH since some folks (me, anyway) don't know what they are without looking them up.

In this case, you can avoid them completely by using s/// like so:

$imagefile =~ s|src="/?||;
[download]

Note that I'm using the vertical bars as delimiters so I don't have to escape the slash. Putting "?" after the slash means that the pattern will match it if it's there, but it doesn't have to be there.

In fact, as I read on, I think that all of your construction of $imagefile could be something like this:

$imagefile =~ s|src="/?(.*)"$|/$1|;
[download]

After that, you're checking $path against a bunch of patterns and setting $ImageFilePath according to what you find. For example:

    # check to see if this image file is in the eMedSample path
    if ($path =~ m/([a-zA-Z0-9\_\-\/])+MMS-Apps\/eMedSample/i) {
        $ImageFilePath = "MMS-Apps/eMedSample/WwwRoot" . $imagefile;
    }
[download]

In your regular expression, you escape some characters inside the character class, but that doesn't work as you intend. It's worthwhile to know that \w is the same as [a-zA-Z0-9_], so your whole expression could be:

$path =~ m{[\w/-]+MMS-Apps/eMedSample}i
[download]

Again, I used different delimiters here to avoid having to escape the slashes.

You could use a data structure for the whole if-else ladder you have.

use List::Util qw( first );

my @pattern_prefix = ( { pat => qr{{[\w/-]+MMS-Apps/eMedSample}i,
                         pre => 'MMS-Apps/eMedSample/WwwRoot', },
                       { pat => qr{...}i,
                         pre => '...' }, );

my $match = first { $path =~ $_->{pat} } @pattern_prefix;
$ImageFilePath = $match ? $match->{pre} . $imagefile
               : "Not sure what the path is here: $path\n";
[download]

That might not be easier for you, and if you later have to do something other than a pattern match as a condition, then you'd have to change it back. There's not really anything wrong with the if-elseif-else you have, but they can get to be a problem if they get too big.

# This function in addition to 
sub ReportFileFound {
    my ($filename, $imagefile, $linenum, $bBuildFilePath) = @_;
[download]

Your comment is chopped off. I usually wouldn't want to have that many parameters to a function. At that point, I'd consider passing in a hash ref to it with all the parameters.

# call as ReportFileFound({ filename  => $fn,
#                           imagefile => $if,
#                           linenum   => $ln,
#                           bBuildFilePath => $bBFP, });
sub ReportFileFound {
   my $args_ref = shift;

# use $args_ref->{filename} for $filename
[download]

This isn't really necessary, of course. It's nice, though, since you can pass your parameters in any order, you can more easily support defaults, and they get a name.

    my $aLength = $#FinalImageFileList + 1;
[download]

This might be more clearly written as:

    my $aLength = scalar @FinalImageFileList;
[download]

        foreach $tmpImageFile (@FinalImageFileList) {
            
            #its not here so lets insert it...
            if ($tmpImageFile eq $ImagePath) {
                $bFound = 1;
                last;
            }
[download]

$bFound = scalar first { $_ eq $imagePath } @FinalImageFileList;
[download]

    open hFile, $filename or die "Failed to open $filename\n";
[download]

Use the three argument form of open to avoid the problems you get if $filename is funny. It's a good idea also to report "$!" in the error message so you know what went wrong when it goes wrong.

open hFile, '<', $filename
    or die "Can't read '$filename': $!\n";
[download]

Another thing you should consider is using lexical filehandles instead of global ones. You can put "my $hFile" in place of "hFile", and Perl will provide you with a filehandle. You can pass $hFile around to other subs easily, and it closes automatically when it goes out of scope.

        if ($line =~ m/src="([a-zA-Z0-9\_\-\/])+\.(gif|jp[e]?g|bmp)"/i
+) {
            $line = $&;
            ReportFileFound($saved_name, $line, $linenum, 1);
        }
[download]

There's another one of those variables with the performance penalty. In this case, you can avoid it using capturing parentheses around the whole pattern.

        if ($line =~ m{(src="[\w/-]+\.(?:gif|jp[e]?g|bmp)")/i) {
            $line = $1;
            ReportFileFound($saved_name, $line, $linenum, 1);
        }
[download]

Note that (?:stuff) is non-capturing parentheses. It forms a group, but it won't stuff something in $2 as it would otherwise.

sub CheckForImageReferencesInSQL {
    my($server, $username, $password, $DB) = @_;
    use MSSQL::Sqllib;
[download]

Note that the use "happens" at compile time regardless of whether the sub is called. Some people like to put them in the sub the way you have. There's a discussion of this in Code style advice: where to put "use" statements?

    foreach $QueryResult (@QueryResult) {
        foreach $row (keys %$QueryResult) {
[download]

I think what you're calling a $row here is actually a field within the row. Having a variable named $QueryResult and another one also named @QueryResult can be confusing, but it's not too bad in this case since they both live and die close together.

@ARGV = (".") unless @ARGV;
[download]

I usually put the program at the top and the subs under it, but that's just style.

    unlink $g_logfile;
[download]

I'd add also:

    or die "Can't unlink '$g_logfile': $!";
[download]

open hLOGFILE, ">>$g_logfile" or die "Could not open the file $g_logfi
+le\n";
[download]

Again, I'd write this as:

open hLOGFILE, '>>', $g_logfile
    or die "Could not append to '$g_logfile': $!\n";
[download]

At the top you use Carp, but it doesn't look as if you actually use it anywhere.

In general, if it works, and it keeps working, that's what matters most. I'd strongly suggest you use strict again, though. If you need to have some global variables, that's not the end of the world, but declare them visibly so every reader knows what they are and what they're for. You have a nice "section" at the top for this, but there's only one variable listed, and I'm pretty sure there are more.

I hope this is helpful. I've written a lot here, and I don't have time to polish it all up as I'd like to before posting. If you have any questions about this, go ahead and ask. I'll do my best to clear up anything I've confused.

In addition to kyle's superb suggestions, you might also consider using the special variable $. (INPUT_LINE_NUMBER) instead of $linenum in your sub CheckForImageReferencesInFiles:
Read more... (710 Bytes)

If the code's not too large, post it here. For a more private review based on best practices, look into installing and using Perl::Critic.

Oh wow there's a library that reviews my code?! That's awesome! I check this out as well. Thanks!