A Practical Use for Macros in Perl generated several thoughtful comments. While Aristotle Pagaltzis identified the real semantic difficulty with the code I wanted to write (and mentioned the Null Object pattern, which I always keep in mind), Chas. Owens asked perhaps the best philosophical question:
Why not modify add_txn to reject undefs?
The code in review is:
while (my $stock = $stock_rs->next)
{
my $pe_update = $self->analyze_pe( $stock );
$stock_txn->add( $pe_update ) if $pe_update;
my $cash_yield_update = $self->analyze_cash_yield( $stock );
$analysis_txn->add( $cash_yield_update ) if $cash_yield_update;
}... and the near duplication obscures (to me) the point of the code. Both Aristotle and Chas. are right—perhaps it's clearer to allow $transaction->add to do nothing when it receives nothing. I write "perhaps" because I see the appeal of that change, but I'm not sure I like it.
As usual with my software, the system has a fundamental design principle: either succeed in full or do nothing. It's fine to skip half of the analysis steps if the data just isn't there. (The project as a whole can succeed if it's only 60% correct; the joy of a margin of error. It's a lot more accurate than that.)
I take this principle to mean that robustness is more important than completeness. Skipping bad data and moving on is perfectly fine. The next run may improve transient errors, and catastrophic errors will require human intervention anyhow.
When these principles translate into design, I prefer to handle errors at
the point of detection and not spread error handling throughout the system.
All of these analysis methods should return something. When they
succeed, they return a hash reference mapping column names to values in a
database table. When these methods fail—whether the existing data isn't
sufficient to calculate updated values or something else went wrong—they
return;. As you well know, that's an empty list in list context
and undef in the scalar context of the example code.
Why add nothing to a transaction when I know there's nothing to add? Yes,
add() could check that it has nothing to do and do nothing, and
that's fine, but it seems like that expands the behavior of
add()'s API to include caller errors. Then again, the
add() method must check that each hash reference contains a value
for the transaction's bound primary key, or it will generate buggy output.
I suspect that both Aristotle and Chas. have in mind Postel's Law:
Be generous in what you accept and picky about what you emit.
The result might look something like:
while (my $stock = $stock_rs->next)
{
$stock_txn->add( $self->analyze_pe( $stock ) );
$analysis_txn->add( $self->analyze_cash_yield( $stock ) );
}This change has an advantage: it only necessitates a change in the
add() method. All of the analyze_*() methods can
continue to work as implemented.
Of course, there's a slight performance penalty to doing this. In my case, it's immaterial, but it wouldn't be present with macros. This is an IO-bound application anyhow, and the transaction manager exists to avoid very real, very measured bottlenecks.
Finally, Aristotle's mention of the null object pattern was about real objects, and not methods which return empty lists or hash references. If that's your style, good for you—but it's not mine in this case. While it's not obvious from the small snippets I've posted so far, the responsibility of the analysis methods is smaller in scope than the responsibility of the transaction objects. Coupling transaction management to the analysis methods—in as much that they have to know about transactions to return the right objects—would turn the design of the system inside out. The result would very likely not be an improvement.
