tieregistry file ext search script critique

gebuh has asked for the wisdom of the Perl Monks concerning the following question:

Hi all, my perl is weak, I wrote a script to return a list of file extensions if a couple of values are present in windows registry. It works, but I'm hoping y'all can give me some tips on improving my code.

#!/usr/bin/perl
#  FileExtP.pl will generate a list of files with incorrect settings f
+or "confirm open after download" 
#  and "always show extension" 
# Use to determine if these values are set in registry.
# File extensions will be loaded from a user supplied file.
# 1/18/2009

use strict;
use warnings;

my $Registry;
use Win32::TieRegistry( TiedRef => \$Registry, Delimiter=>"/", 
                        SplitMultis => 1, ArrayValues=>1,
                        qw( REG_SZ REG_EXPAND_SZ REG_DWORD REG_BINARY 
+REG_MULTI_SZ
                            KEY_READ ),
                     );
                     
open (my $FI, "fileExtensionList.txt") || die "couldn't open input fil
+e";
open (my $FO, "> fileOut.txt") || die "couldn't open output file";

$Registry->Delimiter("/");                  # Set delimiter to "/".
my $regKey = $Registry->{"Classes/"};
my (@fileExtArr);
while (<$FI>) {
    # get the extensions
    s/#.*//;            # ignore comments
    next if /^(\s)*$/;  # skip blank lines
    my($Temp)=$_; #Store each line from $FileName to $Temp
    chomp ($Temp); #strip eol character
    push @fileExtArr, $Temp; 
}

 
#testing, only, get all file extensions from HKCR
#my @allClassKeys = keys( %{$regKey} );
#@fileExtArr = grep {/^\..*/} @allClassKeys;
#  foreach(@fileExtArr){
#      print "$_\n";
#  }


my $ctr = 0;
foreach(@fileExtArr){
      # get value of key each extension points to
      my $eachSubKey = $regKey->{"$_"} or warn " $_  $^E\n";
      if($eachSubKey){
          my $keyPtr = $eachSubKey->GetValue("") or warn "$_  $^E\n"; 
+# go to this key for value
          if($keyPtr){
              # check this key for these values
              if(!$regKey->{$keyPtr}->{"AlwaysShowExt"} || !$regKey->{
+$keyPtr}->{"EditFlags"}){
                  $ctr++;
                  print "$ctr    $_     $keyPtr\n";
              }    
          }
      }
}
close($FI);
close($FO);
exit 0;
[download]

Comment on tieregistry file ext search script critique Download Code

Replies are listed 'Best First'.
Re: tieregistry file ext search script critique by jethro (Monsignor) on Jan 27, 2009 at 14:01 UTC
Nothing really wrong with your script so I'll nitpick `next if /^(\s)*$/; # skip blank lines my($Temp)=$_; #Store each line from $FileName to $Temp chomp ($Temp); #strip eol character push @fileExtArr, $Temp;` [download] The parens around \s are unnecessary. $Temp is unnecessary, just continue to use $_ `# go to this key for value ... # check this key for these values` [download] Bad comments. I like comments, but they have to give more information than what you can as easily read from the code itself. If words like 'this' and 'these' show up frequently, the comments likely are wasted time.	[reply] [d/l] [select]
Re: tieregistry file ext search script critique by toolic (Bishop) on Jan 27, 2009 at 22:17 UTC
All minor points: Get more debug information using the special variable, $!, when checking the success of open. Using the 3-argument form of open is also a good practice: `open my $FI, '<', 'fileExtensionList.txt or die "couldn't open input f +ile: $!";` [download] It doesn't look like you use your other filehandle, `$FO`. Instead of using comments for your description, you could use POD: `=head1 Description FileExtP.pl will generate a list of files with incorrect settings for "confirm open after download and "always show extension" =cut` [download] That way, you can get info about your program from the commandline: `$ perldoc FileExtP.pl` [download]	[reply] [d/l] [select]