In the short term, I've reverted your change in my branch, and will
send that upstream to ask.
-R
Jared Johnson wrote:
>
> - 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
