- my $self = shift;
- my $key = shift;
- @_ and $self->{_notes}->{$key} = shift;
- #warn Data::Dumper->Dump([\$self->{_notes}], [qw(notes)]);
- $self->{_notes}->{$key};
+ my ($self,$key,$value) = @_;
+ $self->{_notes}->{$key} = $value if defined $value;
+ return $self->{_notes}->{$key};
I originally considered these functionally identical, but they are not.
The new code, called with, say, $txn->notes('discard',undef), would
result in evaluation as if it were a 'get' method rather than setting
the 'discard' note to undef. That seems quite dangerous. I suggest
either reverting the language back to the '@_ and' model, or else doing
something like:
my ($self,$key,$value) = @_;
$self->{_notes}->{$key} = $value if scalar @_ > 2;
return $self->{_notes}->{$key};
I'd be happy to submit a patch for either sort of language.
Incidentally, I came across this when looking into a bug that was caused
when I did $txn->recipients(grep {$_ ne $rcpt} $txn->recipients) --
expecting it to clear the recipient list if grep returned an empty list.
Instead, it predictably behaved as a get method, e.g. a noop. I
solved this by adding a clear_recipients() method to
Qpsmtpd::Transactions, then thought better of it and added
remove_recipients() instead. I'll try to get around to submitting these
as well if they seem useful :)
-Jared
