On 1/2/09 5:22 PM, Rob Dixon wrote:
> David Newman wrote:
>> Greetings. I have a working script (pasted below) that uses
>> List::Compare to find common lines in two files.
>>
>> Now I am again looking to compare two files but this time exclude any
>> lines in file 1 that appear in file 2. I don't see anything in the
>> List::Compare documentation on how to do this.
>>
>> Thanks in advance for any clues on a simple way to handle this.
>>
>> dn
>>
>> #!/usr/bin/perl -w
>>
>> use strict;
> 
> You should also use the warnings pragma instead of the command-line switch.
> 
>   use warnings;

OK. Why is this better?

> 
>> use List::Compare;
>>
>> my $lfile =  $ARGV[0];
>> my $rfile =  $ARGV[1];
> 
> It would also be nice to make sure that there are in fact two parameters
> 
>   die "The parameters must be the the two files for comparison"
>       unless @ARGV == 2;
> 
>> my $lc;
>> my @union;
> 
> 
> It is best to declare variables as close as possible to their first use. All 
> you
> need here is
> 
>   my ($lfile, $rfile) = @ARGV;

Is this for readability?

I always found it "cleaner", and have heard others say it's preferable,
to declare all variables at the top of the program (although admittedly
I didn't do that even in this short script).



> 
>> # get files
>> open(DAT, $lfile) or die("unable to open");
>>
>> my @Llist = <DAT>;
>>
>> close(DAT);
> 
> You should include the $! variable in the die string so that you know why the
> open failed. I suggest
> 
>   my @llist;
>   {
>     open my $fh, '<', $lfile or die "Unable to open '$lfile': $!";
>     @llist = <$fh>;
>   }
> 
>> open(DAT, $rfile) or die("unable to open");
>>
>> my @Rlist = <DAT>;
>>
>> close(DAT);
> 
> The same applies to the second parameter.
> 
>> # do comparison
>>
>> $lc = List::Compare->new(\...@llist, \...@rlist);
>>
>> @union = $lc->get_union;
> 
> This call doesn't 'find common lines in two files'. It generates a list of 
> lines
> that appear at least once in either file.
> 
>> foreach my $union (@union) {
>>      print "$union";
>> }
> 
> You shouldn't put $union in quotes, and the loop is better written as
> 
>   print for @union;
> 
> 
> It's not clear from your description what new information you want. You say 
> you
> want to 'exclude any lines in file 1 that appear in file 2', but you don't say
> what you want to exclude those lines from.
> 
> I think you must mean either:
> 
> - All records that appear at least once in either file, but not in both. In 
> that
> case you need
> 
>   my @difference = $lc->get_LorRonly;
> 
> - All records that appear in file 1 but not in file 2. To do that you want
> 
>   my @file1only = $lc->get_Lonly;

Yes, that's the option I'm looking for. Thanks; I hadn't understood this
part of the docs.

Thanks too for the coding comments.

dn


> 
> I hope this helps.
> 
> Rob
> 


-- 
To unsubscribe, e-mail: beginners-unsubscr...@perl.org
For additional commands, e-mail: beginners-h...@perl.org
http://learn.perl.org/


Reply via email to