in reply to Re^2: substr in nested foreach loop
in thread substr in nested foreach loop
Not directly related to your issue, but some comments anyway, in no particular order:
while (<FILE1>) { $hline=<FILE1>; push (@hlines,$hline); }
is the same as
@hlines = <FILE1>;
Also,
my $FILE1;
probably doesn't do what you think. $FILE1 is not the same as the FILE1 you use in your open statement. Either change all places where you use a bare FILE1 to $FILE1 (which would make it a lexical filehandle), or just remove the my $FILE1; declaration.
In your open call, you don't have to put quotes around the filename. It is however a good idea to use the three-argument form of open, in case someone tries to use something like >datafile as filename and causes you to overwrite the data file that way. So instead of
open (FILE1, "$f1") or die "Argh! File not opened! $!";
use something like
open (FILE1, "<", $f1) or die "Argh! File not opened! $!";
In your read subroutine, you're using the variable $a without declaring it. With almost any other name this would have caused a compile time error, but $a and $b are special. For that reason, it's best to avoid using either of those names.
$xh, $yh, $zh, $x1, $y1, $z1, $i, $r and $r2 are only used within your dist subroutine, so you should only declare them in there, and not at the global level
read is also the name of a built-in function, so it's usually better to avoid using it for any of your own functions unless you explicitely want to override it
Your shebang line is showing /root/bin/perl as the path to perl. Since /root is usually only accessible by root, this means that you're developing and testing this code as root, which is generally not recommended.
The use subs; is useless here as far as I can tell.
Since the two file reads are really unrelated, you can reuse the same filehandle for both of them
Making the changes above, the code looks something like this:
#!/root/bin/perl use strict; use warnings; sub square; sub read_filename; sub dist; my $FILE; my $f1 = read_filename; open ($FILE, "<", $f1) or die "Argh! File not opened! $!"; my @hlines = <$FILE>; close $FILE; my $f2 = read_filename; open ($FILE, "<", $f2) or die "Argh! Second file not opened! $!"; my @plines = <$FILE>; close $FILE; print @hlines; print @plines; dist; exit; sub dist { my $i=0; my ($r, $r2); foreach my $hline (@hlines) { my $xh = substr $hline,31,8; my $yh = substr $hline,40,8; my $zh = substr $hline,48,8; foreach my $pline (@plines) { print '"',$pline,'": len=',length($pline),"\n"; my $x1 = substr $pline,31,8; my $y1 = substr $pline,40,8; my $z1 = substr $pline,48,8; $i++; $r2=((square($xh-$x1))+(square($yh-$y1))+(square($zh-$z1)) +); $r=sqrt($r2); print "$i. $r2 and square root is $r\n"; if(($r<=2.4)&&($r>=3.6)) { print "match!"} } } print $i; return 1; } sub square { my $num = shift; my $sq = $num * $num; return $sq; } sub read_filename { print "Enter file name: "; my $fn = <STDIN>; chomp $fn; return $fn; }
This code can stil be further improved, but it should be a nice step in the right direction.
|
---|
Replies are listed 'Best First'. | |
---|---|
Re^4: substr in nested foreach loop
by sarani (Sexton) on May 23, 2006 at 03:54 UTC |