in reply to Re: substr in nested foreach loop
in thread substr in nested foreach loop

#! /root/bin/perl use strict; use warnings; use subs; sub square; sub read; sub dist; my $r; my $r2; my $i=0; my $xh; my $yh; my $zh; my $f1; my $FILE1; my $hline; my @hlines; $f1=&read; my $x1; my $y1; my $z1; my $f2; my $FILE2; my $pline; my @plines; $f2=&read; open (FILE1, "$f1") or die "Argh! File not opened! $!"; open (FILE2, "$f2") or die "Argh! Second file not opened! $!"; while (<FILE1>) { $hline=<FILE1>; push (@hlines,$hline); } + while (<FILE2>) { $pline=<FILE2>; push (@plines,$pline); } close FILE2; close FILE1; print @hlines; print @plines; &dist; exit; sub dist { foreach $hline (@hlines) { $xh= substr $hline,31,8; $yh= substr $hline,40,8; $zh= substr $hline,48,8; foreach $pline (@plines) { print '"',$pline,'": len=',length($pline),"\n"; $x1= substr $pline,31,8; $y1= substr $pline,40,8; $z1= substr $pline,48,8; $i++; $r2=((&square($xh-$x1))+(&square($yh-$y1))+(&square($zh-$z +1))); $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; my $sq; $num=shift; $sq = $num * $num; return $sq; } sub read { print "Enter file name:"; $a=<STDIN>; chomp($a); return $a; }

Replies are listed 'Best First'.
Re^3: substr in nested foreach loop
by Crackers2 (Parson) on May 22, 2006 at 15:40 UTC

    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.

      Thank you so much! :) I can see I have a logn way to go, but this really helps...
Re^3: substr in nested foreach loop
by suaveant (Parson) on May 22, 2006 at 14:39 UTC
    Well... I don't know what the problem may be, but maybe you should add a
    print length($hline),"\n";
    before the substr that's dying, just to be sure, maybe even print the line.

    But the main reason I write is to suggest using one unpack instead of three substrs... syntax would be (approximately, not tested)

    ($xh,$yh,$zh) = unpack("x31A8x1A8A8",$hline);
    Pretty sure that would be more efficient.

                    - Ant
                    - Some of my best work - (1 2 3)