Beefy Boxes and Bandwidth Generously Provided by pair Networks
Keep It Simple, Stupid
 
PerlMonks  

Style or Clarity?

by periapt (Hermit)
on Jun 08, 2004 at 20:05 UTC ( [id://362525]=perlquestion: print w/replies, xml ) Need Help??

periapt has asked for the wisdom of the Perl Monks concerning the following question:

I sometimes find that, in the course of processing decisions, I write a construct similar to this ...
... doing something ... $tmp = "some result"; # not an actual value if($tmp eq 'F'){ ; # do nothing, spot held for clarity }elsif($tmp =~ /[MICL]/){ ... # do something ... }else{ ... # do something }
I know there are ways to eliminate the first comparison but I rather find the structure to be clearer. Still, I got to wondering about the NOOP code in the first if. Does Perl even have a NOOP code when it compiles? B::Deparse reported this construct as an empty list
if ($tmp eq 'F') { (); }elsif ...
OK, so is an empty list placed at this point in the bytecode so that when reached, the program evaluates the list? In what context? Or is this just a shorthand display for some NOOP code? Or am I missing some point entirely?

Thanks

Update: Thanks for the great information. Early in my Perl life, I benchmarked this construct a few times to satisfy myself that speed wasn't an issue. In all the instances that I tested, the construct using the NOOP was at least as fast as the original construct so I tend to opt for clarity.

I like the return if ... option although I still get twinges about having multiple exits from a subroutine. Just can't get that structured BASIC out of my soul ;o)

Thanks again.

PJ
We are drowning in information and starving for knowledge - Rutherford D. Rogers
What good is knowledge if you have to pull teeth to get it - anonymous

Replies are listed 'Best First'.
Re: Style or Clarity?
by dragonchild (Archbishop) on Jun 08, 2004 at 20:08 UTC
    Personally, I would do something like:
    my $tmp = return_from_func(); return if $tmp eq 'F'; # Continue as normal

    I also tend to make large use of dispatch tables and intelligent classes, but those can often be overkill for smaller needs.

    ------
    We are the carpenters and bricklayers of the Information Age.

    Then there are Damian modules.... *sigh* ... that's not about being less-lazy -- that's about being on some really good drugs -- you know, there is no spoon. - flyingmoose

    I shouldn't have to say this, but any code, unless otherwise stated, is untested

      Yeah, I do that a lot too. If you don't mind my asking. What exactly are dispatch tables and intelligent classes? I've seen dispatch tables mentioned before and I've always assumed that they were some table of subroutine calls, maybe in a hash? I've never had a clear idea of an intelligent classes. A little OT but it seems like a good time to ask.

      PJ
      We are drowning in information and starving for knowledge - Rutherford D. Rogers
      What good is knowledge if you have to pull teeth to get it - anonymous
        A dispatch table is a data structure whose values are subroutine references. In C, this would generally be an array of funcp's. In Perl, it's generally a hash (though it can be an array).

        An intelligent class, in this context, is more of a group of classes. It's best explained with an example. Let's use the one from above - a bunch of menu items that may be displayed, depending on some set of external conditions.

        The immediately obvious solution would be to create some massive if-else structure to handle every obvious possibility. After about 3-4 distinct options, this becomes unmaintainable.

        The next solution most people arrive at is to create some massive data structure and some control function that will "walk the tree", so to speak. This is more manageable, but starts to become problematic around 10-20 distinct options. It also has issues if there is more than one set of external conditions.

        Intelligent classes is a variant of the latter solution. You still have the control function, but it's much simpler. It generally would look something like:

        my @list_of_classes = ( ... ); foreach my $class (@list_of_classes) { next unless $class->should_render( \%conditions1, \%conditions2, ... ); $class->render; }

        You then have a bunch of classes that all provide the should_render() and render() methods. They can be related through inheritance, or not. But, what they all do is determine for themselves whether or not they should render. This means that the conditions are broken out into the units that are smaller and self-contained, thus easier to maintain. When I use this method, I get up to 50 or 60 distinct options with 2-3 sets of external conditions and have no problem.

        ------
        We are the carpenters and bricklayers of the Information Age.

        Then there are Damian modules.... *sigh* ... that's not about being less-lazy -- that's about being on some really good drugs -- you know, there is no spoon. - flyingmoose

        I shouldn't have to say this, but any code, unless otherwise stated, is untested

Re: Style or Clarity?
by dave_the_m (Monsignor) on Jun 08, 2004 at 21:34 UTC
    B::Deparse reported this construct as an empty list
    if ($tmp eq 'F') { (); }elsif ...
    OK, so is an empty list placed at this point in the bytecode so that when reached, the program evaluates the list? In what context? Or is this just a shorthand display for some NOOP code?
    The actual bytecode at this point is a single 'stub' op, whose action is to return undef in a scalar context and nothing otherwise. In your example, it's called in a void context. The empty list happens to compile to the same op.

    Dave.

      Ah, so that's it. :o) Thanks

      PJ
      We are drowning in information and starving for knowledge - Rutherford D. Rogers
      What good is knowledge if you have to pull teeth to get it - anonymous
Re: Style or Clarity?
by Zaxo (Archbishop) on Jun 08, 2004 at 21:52 UTC

    That sort of logical structure can be recast as a case switch, ala perlsyn,

    SWITCH: { $tmp eq 'F' and last SWITCH; $tmp =~ /[MICL]/ and do { # Some things... } , last SWITCH; # do default else thing }
    At the expense of comprehensibility, you can omit the last clause from a statement to have processing continue.

    After Compline,
    Zaxo

Re: Style or Clarity?
by sgifford (Prior) on Jun 08, 2004 at 20:29 UTC
    If you're concerned that the clearer version should be slower, you should benchmark it:
    Benchmark: timing 100000 iterations of clarity, speed... clarity: 2 wallclock secs ( 1.74 usr + 0.00 sys = 1.74 CPU) @ 57 +471.26/s (n=100000) speed: 2 wallclock secs ( 1.78 usr + 0.00 sys = 1.78 CPU) @ 56 +179.78/s (n=100000)

    So it appears that, at least in this test, the clearer version is a little faster. Your tests can simulate your environment better, but it looks like both chunks of code are really close to the same speed, and you should code however you find most clear and enjoyable. :)

    Update: Abigail-II's right, this benchmark is a little off, and in fact a first attempt to correct it makes speed slightly faster, though it's still within a few percent. Still, you can easily see how to modify this to benchmark your actual functions and see if there is enough of a speed difference to sacrifice clarity.

      Considering that clarity ('F') returns 0, and speed ('F') returns 2, I don't think the benchmark is valid.

      Abigail

Re: Style or Clarity?
by Grygonos (Chaplain) on Jun 08, 2004 at 20:28 UTC

    Agreed dragonchild I often find myself (in validation subs) doing the following

    foreach (@_) { return 0 if !$_; }
    Simply disallowing undefined values to be passed for validation. Your caller will need to be mindful of the return value, so that it can re-request input...but that's just one example of where I find this type of programming useful.

Re: Style or Clarity?
by CountZero (Bishop) on Jun 08, 2004 at 20:29 UTC
    Whatever works for you is OK, although maintainers of your code may later grumble.

    I don't think that context matters for the evaluation of the empty list in your code as any results are thrown away anyhow.

    One way of testing it is wrapping your code in a subroutine and making sure that this part gets executed. As you know the return value of a sub is (unless you use an explicit return) the value of the last code evaluated. Check the return value of the sub and you will know it. To be on the safe side, call your subroutine in a scalar context and a list context and see if it matters.

    CountZero

    "If you have four groups working on a compiler, you'll get a 4-pass compiler." - Conway's Law

Re: Style or Clarity?
by FoxtrotUniform (Prior) on Jun 09, 2004 at 01:54 UTC
    if($tmp eq 'F'){ ; # do nothing, spot held for clarity }elsif($tmp =~ /[MICL]/){ ... # do something ... }else{ ... # do something } }

    If you have a few different cases, an if/elsif/else chunk like this one is fine, but if you have a lot of possibilities, you should consider building a dispatch table: fill a hash with subrefs and do something like (untested)

    $tmp = $some_result; my $handler = $handlers{$tmp}; &$handler(...) if $handler; # might be undef, as in 'F' case

    This has the disadvantage of failing silently if you get an unexpected value in $tmp; you could fix it by giving 'F' a do-nothing subroutine and raising an error if $handler is undefined.

    --
    F o x t r o t U n i f o r m
    Found a typo in this node? /msg me
    % man 3 strfry

      This is a good tip. My thanks to you and dragonchild for suggesting it. Fortunately, my decisions have, until now, been short although I did have a 27 element decision tree that could use a dispatch table.

      Thanks

      PJ
      We are drowning in information and starving for knowledge - Rutherford D. Rogers
      What good is knowledge if you have to pull teeth to get it - anonymous
      Another trick if you must must must have the if statements, is to replace the conditional with a subroutine. i.e (assuming F stands for failed)...
      if(failed($status)) { .... } else if(!responseNotFailed($status)) { } else { }

      Bart: God, Schmod. I want my monkey-man.

Re: Style or Clarity?
by baruch (Beadle) on Jun 09, 2004 at 03:14 UTC

    Consider whose processing time is more valuable - yours, or the computer's. Unclear code leads to confusion and errors, which waste enormous amounts of time. If your time or memory constraints are really that critical, you probably need to use C or assembly language. Otherwise, a few NOOP's are well worth the gain in clarity. Let the computer do the extra work...


    בּרוּך
Re: Style or Clarity?
by halley (Prior) on Jun 09, 2004 at 17:40 UTC
    It's kind of late for me to join this thread, but here it is, anyway.

    The best style is the one with the most clarity.

    If it's not clear to the reader, it's not a good style. Forget about matching plaids with patterns or curly braces that line up... if your co-workers (present and future) can read it, it is all good.

    --
    [ e d @ h a l l e y . c c ]

Log In?
Username:
Password:

What's my password?
Create A New User
Domain Nodelet?
Node Status?
node history
Node Type: perlquestion [id://362525]
Approved by sacked
Front-paged by broquaint
help
Chatterbox?
and the web crawler heard nothing...

How do I use this?Last hourOther CB clients
Other Users?
Others avoiding work at the Monastery: (3)
As of 2024-03-29 02:24 GMT
Sections?
Information?
Find Nodes?
Leftovers?
    Voting Booth?

    No recent polls found