use strict; use warnings; use File::Basename; use Text::ParseWords; # I moved your sub to the bottom. I guess that's a matter of personal taste, # at any rate I like it better that way. # I included this line so we get to call getValue without parens or that # ugly &sigil. use subs qw(getValue); # Three-argument open() is generally preferred. # Also, file handles can go into scalars and that's actually the # recommended way to go about things these days. # Error checking for opening now happens right after the attempt to # open # the file, instead of after the while(){} block. open my $file, "<", $ARGV[0] or die "Couldn't open file '$ARGV[0]': $!.\nDid you specify a valid file?"; # I've moved the three variables here, so that we don't lose their # values through iterations. # Also, why are you using arrays when they clearly work as scalars? # I changed that. # my (@ID,@ISIN,@SYMBOL); my ($ID, $ISIN, $SYMBOL); # Fixed indentation. # Also, I usually don't like working with $_. # There's too many subtleties and pitfalls. while (my $line = <$file>) { # I don't care about newlines chomp $line; if ($line =~ m/ID:/) { $ID = getValue $line ; } if ($line =~ m/ISIN:/) { $ISIN = getValue $line ; } if ($line =~ m/Symbol:/) { $SYMBOL = getValue $line; } # When all of $ID, $ISIN, and $SYMBOL are set, we can # print this stuff. if (defined($ID) and defined($ISIN) and defined($SYMBOL)) { # You wanted a newline, right? # Okay, I'll give you a newline! print "$ID:$ISIN:$SYMBOL\n"; # There it is ----------^^ # You still prefer to go with printf? Okay. # printf "%s:%s:%s\n", $ID, $ISIN, $SYMBOL; # And reset the stuff. undef $ID; undef $ISIN; undef $SYMBOL; } # Same thing here - you just want a scalar that holds all the # values. But do you really need it? I don't think so. # my @FULLVAR = (@ID,@ISIN,@SYMBOL); # Same goes for these - what are these sweeties doing here? #my $num = 0; #my $count = 0; # I'm not even sure what this is supposed to do, # or why you use printf but none of its formatting capabilities. # Also, we've moved the printing part to inside the new if(){} # block so I'm gonna comment this out. # foreach (@ID,@ISIN,@SYMBOL) { printf "$_:"; } } # This curly bracket ends the while(){} block... # But then what's going on here? # I'm guessing it's a copy-paste error, because I get an error: # Unmatched right curly bracket at ... # Also, we've moved this piece to the line where we try to open the # file, so begone with it! #} else { # print "You need to specify an input file \n"; # print "Usage : ".basename($0)." difffile.txt \n"; # exit; #} sub getValue { # Are you sure this regular expression is correct? # It removes all spaces, but in the example you provided # you showed that you wanted to keep the space in the # Symbol field. # $_ =~ s/\s//g; # My guess is that you just want to trim leading and trailing # spaces from the value, so let's do that. And where would these # spaces occur? Right after the colon, so let's make it part # of the string to split on. my $line = shift; my ($name, $value) = split(/:\s*/, $line); chomp $value; return $value; } #### G:\>perl x.pl G:\>perl nonexistant.pl Can't open perl script "nonexistant.pl": No such file or directory G:\>perl x.pl diff.txt QYQ:SE0004017929:LUP2L 100OHM R1M:SE0004018539:TLS2K 50OHM QNF:SE0003990183:MINILONG OMX A O QX8:SE0004017440:ALF2K 160OHM NC0:SE0003842137:BOL2K 170OHM NEV:SE0003843069:NOK2K 90OHM G:\>