[Pdx-pm] sub calling logic

[Ovid]

--- "Roderick A. Anderson" <raanders at acm.org> wrote:
> How about placing the
> subroutine call anywhere I think it might fit and have it decide if
> it
> needs to do anything.

Interesting timing.  I just sent an email to a few friends asking for
some feedback on OO tips I was putting together and something very
similar to this was in there.

Rewriting your examples into something we might actually see, you asked
if this:

  if ($error) {
    log_error($error);
  }

Should be replaced with:

  sub log_error {
    my $error = shift;
    return unless $error;
    ...
  }

The answer depends upon whether or not log_error() is logically
dependent on $error.  If it is the case that log_error() cannot proceed
unless $error is true, then your decision is a good one.  The test of
$error belongs in log_error() and not outside of it.  Obviously, there
is a dependency here, so you're fine.

If, however, you have a subroutine named write_current_orders(), you
might think that this is okay:

  sub write_current_orders {
    my ($file, @orders) = @_;
    return unless @orders;
    ...
    _write_formatted_data($file, $data);
  }

In fact, it probably is okay.  However, what if your requirements
change and you want to write out a "no orders received at $time"
message if you have no orders?  Then you might think you have to test
orders:

  if (@orders) {
      write_current_orders($file, @orders);
  }
  else {
      write_no_order_entry($file);
  }

In this case, we probably don't even want to do that as we can still
push this test into the first sub (since "no orders received" is
logically dependent on whether or not we have current orders).

  sub write_current_orders {
    my ($file, @orders) = @_;
    my $data = _format_data(@orders);
    if ($data) {
      _write_formatted_data($file, $data);
    }
    else {
      _write_formatted_data($file, "no orders received");
    }
  }

Here, we've managed to eliminate a useless subroutine and the calling
code merely calls the write_current_orders() subroutine without
worrying about extra logic.  Once again, we have a win, but sometimes
we have to think about dependencies to see how.

In short, while it's not always the case that we can push logic down
that far, the reality is, the closer logic is to the code that actually
is dependent on it, the fewer bugs that we will have.

Cheers,
Ovid

=====
Silence is Evil            http://users.easystreet.com/ovid/philosophy/indexdecency.htm
Ovid                       http://www.perlmonks.org/index.pl?node_id=17000
Web Programming with Perl  http://users.easystreet.com/ovid/cgi_course/