Re: Identify new pairs in an array

Andy <burdrs@xxxxxxxxx> wrote:


My attempt is shown below. I think that I am very close.


But you are not going to tell us what little bit it is that is missing?

Leaving us to figure it out for ourselves from the (quite horrid) code?

Many readers will simply move on to answering someone else's question
if you don't make it easier for them to answer yours...


Any
suggestions would be welcome.


I have two meta-suggestions:

1) Please see the Posting Guidelines that are posted here frequently.

2) Adopt a coding style that won't make people spit.


#!/usr/bin/perl -w
use warnings;


Keep the pragma, lose the switch.


[ declarations below rearranged into groups I can comment on ]

my %hash1;
@hash1{@uniq} = ();

my @hash1;


Those are 2 *different* variables, and your code does not
make use of either one of them!

So what are they there for?


my $d;
my $e;
my $f;
my $g;


One-character variable names suck.

One-character variable names suck even more when they
are "global" variables.

Try to chose meaningful variable names.


You should limit the scope of variables to the smallest possible scope.

So you should be declaring your variables at their first use, not
all at the top of the program.


foreach my $i ( 0 .. $#uniq-1 ) {
foreach my $j ( $b .. $#uniq ) {


There's a nice style, spaces between significant parts.


while ($done==0){


Why did that nice style suddenly disappear?

while ( $done == 0 ) {
or
until ( $done ) {


foreach my $p ( 0 .. $k ) {


Nice style now reappears...


if( ($pairs[$p][0]=$f or $pairs[$p][1]=$f) and
($pairs[$p][0]=$g or $pairs[$p][1]=$g) ){


.... now it is gone again!

I'm getting dizzy. :-)


Did you really want _assignments_ in your conditional, or did you
instead mean to use comparisons (==) ?

And if so, did you want to do a numeric comparison (==) or
a string comparison (eq) ?


$done=1;
}


You should indent the contents of blocks, so that the structure of the
program is easier to see.


$done=1;
}


@bud1=( [$d,$e] );
push(@pairs, @bud1);


You don't need the @bud1 temporary variable.

push( @pairs, [$d,$e] );


print $d." ".$e;
print "\n";


Using interpolation instead of literal concatenation nearly always
results in something that is easier to read and understand.

print "$d $e\n"



Note that I made no attempt to understand what your code is
doing or how to fix it, because you've made that too hard.


--
Tad McClellan SGML consulting
tadmc@xxxxxxxxxxxxxx Perl programming
Fort Worth, Texas
.

Relevant Pages

  • Re: c99/c++ localised variable definition
    ... When variables are used only in a limited scope I find it better to ... Forcing variable declarations to the top of the function means extra ... > reason why things like ancillary counter variables seem reasonable, ... you can combine the decl and the initialization. ...
    (freebsd-arch)
  • Re: Making C better (by borrowing from C++)
    ... there's no way to be certain unless declarations are restricted not ... merely to the top of the scope, but to the top of the function. ... line by line for the matching declaration; the context switch between ... the two search modes costs at least as much as the nominally more ...
    (comp.lang.c)
  • Re: About String
    ... My idea is that every statement list is a scope. ... What is so special in statement lists? ... and to me declarations intermixed with statements look similarly ... You declare something inside a loop body, ...
    (comp.lang.ada)
  • Re: Java needs "goto" (was Re: hi)
    ... declarations are collected at the start of the method, ... the whole thing and starting from scratch. ... Too much work to be done and too lil time to do it. ... Played with this scope thing in just about ...
    (comp.lang.java.programmer)