G'day UpTide,

Writing a condition, on one line, that is almost a thousand characters long, and contains over a hundred unexplained, numeric constants, is utterly ridiculous! It's a maintenance nightmare; extremely error-prone; and a disaster just waiting to happen.

For a start, when you have complex conditions, spread them over multiple lines, and line up the conditions such that they are easy to read, e.g.

if ( $matrix[0][4][0] == 1 and $matrix[0][4][1] == 1 and $matrix[0][4][2] == 1 ... ) { # action if TRUE }

Next, although those numeric constants might be meaningful to you now, while you're writing the code and it's fresh in your mind, they won't be when you return to this code later; they most certainly will not be for anyone looking at this code for the first time. There are many ways to give these constants meaningful names: the builtin pragma, constant, is one option; the CPAN module, enum, is another. The SEE ALSO sections of both of those have other suggestions; a "CPAN search for 'constant'" produces hundreds of results.

Using meaningful names should make your code self-documenting; however, you may want to add comments (e.g. to explain why you're testing for exact equality with "1", as opposed to, say, testing for any TRUE value).

Instead of putting that much code in the middle of your script, use a subroutine. I think you'll agree this is much clearer:

print "it worked" if matrix_ok(\@matrix);

In this instance, instead of dozens of equality tests, you can use loops similar to what you've already written. For efficiency, I wouldn't repeatedly redeclare and redefine the fixed indices. Here's an untested example:

sub matrix_ok { my ($matrix) = @_; { my $i = 4; for my $j (0 .. 2, 4 .. 6) { for my $k (0 .. 2) { return 0 unless $matrix->[$i][$j][$k] == 1; } } } { my $j = 4; for my $i (0 .. 2, 5, 6) { for my $k (0 .. 2) { return 0 unless $matrix->[$i][$j][$k] == 1; } } } return 1; }

Note that I didn't use meaningful names, because I have no idea what all those numbers mean. If I did know what they meant, I might write code like this:

... for my $k (MIN_DEPTH .. MAX_DEPTH) { ...

See also (functions such as any() and all() in): the builtin module List::Util; and the CPAN modules, Perl6::Junction and Quantum::Superpositions.

— Ken


In reply to Re: 36 Conditions in If Statement [sendhelp]] by kcott
in thread 36 Conditions in If Statement [sendhelp]] by UpTide

Title:
Use:  <p> text here (a paragraph) </p>
and:  <code> code here </code>
to format your post, it's "PerlMonks-approved HTML":



  • Posts are HTML formatted. Put <p> </p> tags around your paragraphs. Put <code> </code> tags around your code and data!
  • Titles consisting of a single word are discouraged, and in most cases are disallowed outright.
  • Read Where should I post X? if you're not absolutely sure you're posting in the right place.
  • Please read these before you post! —
  • Posts may use any of the Perl Monks Approved HTML tags:
    a, abbr, b, big, blockquote, br, caption, center, col, colgroup, dd, del, details, div, dl, dt, em, font, h1, h2, h3, h4, h5, h6, hr, i, ins, li, ol, p, pre, readmore, small, span, spoiler, strike, strong, sub, summary, sup, table, tbody, td, tfoot, th, thead, tr, tt, u, ul, wbr
  • You may need to use entities for some characters, as follows. (Exception: Within code tags, you can put the characters literally.)
            For:     Use:
    & &amp;
    < &lt;
    > &gt;
    [ &#91;
    ] &#93;
  • Link using PerlMonks shortcuts! What shortcuts can I use for linking?
  • See Writeup Formatting Tips and other pages linked from there for more info.