large expression formating

Dr.Altaica has asked for the wisdom of the Perl Monks concerning the following question:

I'm working a program to convert a pixel based map(in a two dimintinal array) to a vector format(actuly it's goingto be POVray file)

The problem I'm haveing is that the expressions are geting extramly long and I don't know of any styles to use that keeps the expressions easy to work with and fit in with the rest of the code.

Long messy example code hidden behind a read more tag.

my @map = (
    ["#","#","#","#"," "," "," ",],
    ["#"," "," ","#"," "," ","#",],
    ["#"," "," ","#"," "," ","#",],
    ["#","#","#","#"," "," ","#",],
    [" "," "," "," "," "," ","#",],
    ["#","#","#","#","#","#","#",],
    ["#"," "," ","#"," "," ","#",],
    ["#"," "," ","#"," "," ","#",],
    ["#","#","#","#","#","#","#",],
    ["#"," "," ","#"," "," "," ",],
    ["#"," "," ","#"," "," "," ",],
);

####-----------------------------------------------
sub test1{ 
    my($xCur, $yCur, $wall)= @_;

    return "" if $yCur-1 < 0 || $xCur-1 < 0 || !defined($map[$yCur-1])
+;
    return "" if !defined($map[$yCur-1][$xCur-1]);

    return $map[$yCur-1][$xCur-1] eq $wall;
};
#--------------------------------------------------
sub test2{ 
    my($xCur, $yCur, $wall)= @_;

    return "" if $yCur-1 < 0 || $xCur < 0 || !defined($map[$yCur-1]);
    return "" if !defined($map[$yCur-1][$xCur]);

    return $map[$yCur-1][$xCur] eq $wall;
};
#--------------------------------------------------
sub test3{ 
    my($xCur, $yCur, $wall)= @_;

    return "" if $yCur-1 < 0 || $xCur+1 < 0 || !defined($map[$yCur-1])
+;
    return "" if !defined($map[$yCur-1][$xCur+1]);

    return $map[$yCur-1][$xCur+1] eq $wall;
};
####-----------------------------------------------
sub test4{
    my($xCur, $yCur, $wall)= @_;

    return "" if $yCur < 0 || $xCur-1 < 0 || !defined($map[$yCur]);
    return "" if !defined($map[$yCur][$xCur-1]);

    return $map[$yCur][$xCur-1] eq $wall;
};
#--------------------------------------------------
sub test5{ 
    my($xCur, $yCur, $wall)= @_;

    return "" if $yCur < 0 || $xCur < 0 || !defined($map[$yCur]);
    return "" if !defined($map[$yCur][$xCur]);

    return $map[$yCur][$xCur] eq $wall;
};
#--------------------------------------------------
sub test6{ 
    my($xCur, $yCur, $wall)= @_;

    return "" if $yCur < 0 || $xCur+1 < 0 || !defined($map[$yCur]);
    return "" if !defined($map[$yCur][$xCur+1]);

    return $map[$yCur][$xCur+1] eq $wall;
};
####-----------------------------------------------
sub test7{ 
    my($xCur, $yCur, $wall)= @_;

    return "" if $yCur+1 < 0 || $xCur-1 < 0 || !defined($map[$yCur+1])
+;
    return "" if !defined($map[$yCur+1][$xCur-1]);

    return $map[$yCur+1][$xCur-1] eq $wall;
};
#--------------------------------------------------
sub test8{ 
    my($xCur, $yCur, $wall)= @_;

    return "" if $yCur+1 < 0 || $xCur < 0 || !defined($map[$yCur+1]);
    return "" if !defined($map[$yCur+1][$xCur]);

    return $map[$yCur+1][$xCur] eq $wall;
};
#--------------------------------------------------
sub test9{ 
    my($xCur, $yCur, $wall)= @_;

    return "" if $yCur+1 < 0 || $xCur+1 < 0 || !defined($map[$yCur+1])
+;
    return "" if !defined($map[$yCur+1][$xCur+1]);

    return $map[$yCur+1][$xCur+1] eq $wall;
};


# x >    ,y V
# &horiwalls() Goes throght @map and print out a list of horizontal(x/
+1st axis) walls
sub horiwalls {
    my $wall = " ";
#    my $xCur = 0;
#    my $yCur = 0;

    my $mmy = scalar(@map);
    for (my $yCur =0; $yCur < $mmy; $yCur++) {
        my $mmx = scalar(@{$map[$yCur]});
        for (my $xCur =0; $xCur < $mmx; $xCur++) {
            my $tempwall = '.';#temp for testing
            $wall = $map[$yCur  ][$xCur  ];
            $wall = undef if ($wall eq " ");
########################            if ((!&test2($xCur, $yCur, $wall) 
+&& !&test3($xCur, $yCur, $wall) && &test5($xCur, $yCur, $wall) && &te
+st6($xCur, $yCur, $wall) && !&test9($xCur, $yCur, $wall))&& (!&test1(
+$xCur, $yCur, $wall) &&  !&test7($xCur, $yCur, $wall) ) || (&test1($x
+Cur, $yCur, $wall) && &test4($xCur, $yCur, $wall) && &test7($xCur, $y
+Cur, $wall) && !&test8($xCur, $yCur, $wall))) {#end iftest
            if (
( !&test3($xCur, $yCur, $wall) && &test5($xCur, $yCur, $wall) && &test
+6($xCur, $yCur, $wall) && !&test9($xCur, $yCur, $wall))&& 
(
(!&test2($xCur, $yCur, $wall) && ((!&test1($xCur, $yCur, $wall) &&  !&
+test7($xCur, $yCur, $wall) ) || 
                                 (&test1($xCur, $yCur, $wall) && &test
+4($xCur, $yCur, $wall) && &test7($xCur, $yCur, $wall) && !&test8($xCu
+r, $yCur, $wall))))||
!&test1($xCur, $yCur, $wall) &&
&test2($xCur, $yCur, $wall) &&
!&test4($xCur, $yCur, $wall) &&
!&test7($xCur, $yCur, $wall) &&
!&test8($xCur, $yCur, $wall))
) {#end iftest
                $tempwall = "[";#X:$xCur Y:$yCur\n";
            };#end if for wall begian

            if (
#+++++++++++
(!&test1($xCur, $yCur, $wall)&&
  &test4($xCur, $yCur, $wall)&&
  &test5($xCur, $yCur, $wall)&&
 !&test7($xCur, $yCur, $wall)&&
 !&test8($xCur, $yCur, $wall)
)&&
(
 (
  &test2($xCur, $yCur, $wall)&&
 !&test3($xCur, $yCur, $wall)&&
 !&test9($xCur, $yCur, $wall)
 )||
 (!&test2($xCur, $yCur, $wall)&&
   &test6($xCur, $yCur, $wall)&&
   &test9($xCur, $yCur, $wall)
 ))

#+++++++++++
               ){
                $tempwall = ']';
            };# end if for end of wall
            print $tempwall;
###########################
=pod
if (&test1($xCur, $yCur, $wall)) {print "1"}else{  print " " };
if (&test2($xCur, $yCur, $wall)) {print "2"}else{  print " " };
if (&test3($xCur, $yCur, $wall)) {print "3\n"}else{  print " \n" };

if (&test4($xCur, $yCur, $wall)) {print "4"}else{  print " " };
if (&test5($xCur, $yCur, $wall)) {print "5"}else{  print " " };
if (&test6($xCur, $yCur, $wall)) {print "6\n"}else{  print " \n"};

if (&test7($xCur, $yCur, $wall)) {print "7"}else{  print " " };
if (&test8($xCur, $yCur, $wall)) {print "8"}else{  print " " };
if (&test9($xCur, $yCur, $wall)) {print "9-\n"}else{  print " -\n" };
=cut    

###########################
        };print "\n"
    };
};#end &horiwalls
#---------------------------------------------------
# &horiwall() Goes throght @map and print out a list of horizontal(x/1
+st axis) walls
sub horiwall {
    my $wall = " ";
#    my $xCur = 0;
#    my $yCur = 0;
    my $mmy = scalar(@map);

# can use this becouse of Autovivification     for (my $xCur =0; $xCur
+ < scalar(@map); $xCur++) {
    for (my $yCur =0; $yCur < $mmy; $yCur++) {
# can use this becouse of Autovivification        for (my $yCur =0; $y
+Cur < scalar(@{$map[$yCur]}); $yCur++) {
        my $mmx = scalar(@{$map[$yCur]});
        for (my $xCur =0; $xCur < $mmx; $xCur++) {
            print $map[$yCur  ][$xCur  ];
        };
        print "\n";
    };
};#end &horiwall

#---------------------------------------------------
# &vertwalls() Goes throght @map and print out a list of vertical wall
+s
sub vertwalls {
};#end &vertwalls

&horiwalls();
[download]

|\_/|
/o o\
(>_<)
 `-'

Comment on large expression formating Download Code

Replies are listed 'Best First'.
Re: large expression formating by Aristotle (Chancellor) on Oct 20, 2002 at 19:51 UTC
You have false laziness and not enough hubris. (Which means you are copy-pasting where you should be abstracting.) The fact you are calling your subs `test1`, `test2`, `test3` etc should immediately set off the alarms. As a minor note, whenever you want to return "false" from a function in Perl, you should do that with a blank `return`. You should neither `return 0` nor `return ""` - because these return a one-element list, which while false in scalar context is true in list context. A blank `return` will return `undef` in scalar and an empty list in list context, which is what you want. `my @displacement = ( [-1, -1], [-1, 0], [-1, +1], [ 0, -1], [ 0, 0], [ 0, +1], [+1, -1], [+1, 0], [+1, +1], [-1, -1], ); sub check_coord { my ($x, $y, $wall, $coord)= @_; for($displacement[$coord - 1]) { $x += $_->[0]; $y += $_->[1]; } return if $y < 0 or $x < 0 or not defined( $map[$y] or $map[$y][$x] ); return $map[$y][$x] eq $wall; }` [download] and then you can use grep - this: `if ((!&test2($xCur, $yCur, $wall) && !&test3($xCur, $yCur, $wall) && &test5($xCur, $yCur, $wall) && &test6($xCur, $yCur, $wall) && !&test9($xCur, $yCur, $wall))&& (!&test1($xCur, $yCur, $wall) && !&test7($xCur, $yCur, $wall) ) \|\| (&test1($xCur, $yCur, $wall) && &test4($xCur, $yCur, $wall) && &test7($xCur, $yCur, $wall) && !&test8($xCur, $yCur, $wall)))` becomes `if(grep { check_coord($xCur, $yCur, $wall, $_) } 2..8) {` ~~since~~ grep in scalar context returns the number of matches. You can apply the same translation to the rest of your code. I recommend you read Mark-Jason Dominus' excellent Program Repair Shop and Red Flags article series on Perl.com. Makeshifts last the longest.	[reply] [d/l] [select]
Re: Re: large expression formating by Dr.Altaica (Scribe) on Oct 20, 2002 at 22:42 UTC
Thanks about return. That means I need replace: `return $map[$yCur-1][$xCur-1] eq $wall;` with `return if!($map[$yCur-1][$xCur-1] eq $wall);`? but when are you realy going to have a truth in list context? hmm I wounder is perl6 with have a list function.... I don't know much about grep but that apears to be equivalent to `if( &test2($xCur, $yCur, $wall) && &test3($xCur, $yCur, $wall) && &test4($xCur, $yCur, $wall) && &test5($xCur, $yCur, $wall) && &test6($xCur, $yCur, $wall) && &test7($xCur, $yCur, $wall) && &test8($xCur, $yCur, $wall))` [download] It gets me this ((((..( (..(..( (..(..( ((((..( ......( ((((((( (..(..( (..(..( ((((((( (..(..( ((((((( Not this. (.).... ....... ....... (..)... ....... (.)(.). ....... ....... .().(.) ....... ....... and I don't see you doing anything to prevent wraping around the array? \|\_/\| /o o\ (>_<) `-'	[reply] [d/l] [select]
Re^3: large expression formating by Aristotle (Chancellor) on Oct 20, 2002 at 23:41 UTC
No no. `return $map[$yCur-1][$xCur-1] eq $wall;` is fine. But, ouch. You're right about my simplification with grep, I looked far too briefly. Now let's see.. I always get confused by what conditions to use for grep in boolean context, so I'll think aloud. `!$cond1 and !$cond2` means that neither of them must be true. So the number of matches when testing for truth must be zero. That's `not grep { $_ } $cond1, $cond2`. So I was using the wrong one to begin with. And I also overlooked that you weren't simply testing all coordinates for falseness - unraveled, it looks like this: `if ( ( !&test2($xCur, $yCur, $wall) && !&test3($xCur, $yCur, $wall) && &test5($xCur, $yCur, $wall) && &test6($xCur, $yCur, $wall) && !&test9($xCur, $yCur, $wall) ) && ( !&test1($xCur, $yCur, $wall) && !&test7($xCur, $yCur, $wall) ) \|\| ( &test1($xCur, $yCur, $wall) && &test4($xCur, $yCur, $wall) && &test7($xCur, $yCur, $wall) && !&test8($xCur, $yCur, $wall) ) )` [download] That looks decidedly more complex than a simple grep. But since the inner parens only contain &&s, the first two can be combined: `if ( ( !&test2($xCur, $yCur, $wall) && !&test3($xCur, $yCur, $wall) && &test5($xCur, $yCur, $wall) && &test6($xCur, $yCur, $wall) && !&test9($xCur, $yCur, $wall) && !&test1($xCur, $yCur, $wall) && !&test7($xCur, $yCur, $wall) ) \|\| ( &test1($xCur, $yCur, $wall) && &test4($xCur, $yCur, $wall) && &test7($xCur, $yCur, $wall) && !&test8($xCur, $yCur, $wall) ) )` [download] Now that looks almost like a datastructure, so let's make it one. `my %test1 = ( 2 => 0, 3 => 0, 5 => 1, 6 => 1, 9 => 0, 1 => 0, 7 => 0, ); my %test2 = ( 1 => 0, 4 => 0, 7 => 1, 8 => 0, );` [download] Now we have good reason to modify the behaviour of the `check_coordinates` function: it isn't returning true/false, it's returning a comparison value. So have it `return 0 if $lots_of_checks;` and `return 1 if $map[$y][$x] eq $wall;`. Then we can rewrite that `if` as: `if( not(grep { check_coord($xCur, $yCur, $wall, $_) ne $test1{$_} } ke +ys %test1) or not(grep { check_coord($xCur, $yCur, $wall, $_) ne $test2{$_} } + keys %test2) ) { }` [download] That looks bulky, but remember it's as bulky as it's going to get. If you need to add more conditions, you add them in the hashes, not the `if`. And rather than `%test1` and `%test2` you should use meaningful names - maybe `%corner_pixels` f.ex or some such, so your variables express your intent. Makeshifts last the longest.	[reply] [d/l] [select]
Re: large expression formating by rinceWind (Monsignor) on Oct 21, 2002 at 10:44 UTC
You might want to take a look at TheDamian's module DFA::Cellular. Your application looks very suitable for it. I was introduced to this module at a talk he gave in London in September.	[reply]