On Jun 4, 2012, at 7:35 PM, Charlie Brady wrote:
>
> On Mon, 4 Jun 2012, Matt Simerson wrote:
>
>> + my $count = $self->connection->notes('unrec_cmd_count') || 0;
>> + $self->connection->notes('unrec_cmd_count',
>> ++$count );
>>
>> Which does increment correctly.
>>
>> This also works (note the parens), which is how it was done before.
>>
>> + my $count = ($self->connection->notes('unrec_cmd_count') || 0) + 1;
>> + $self->connection->notes('unrec_cmd_count',
>> $count );
>>
>> Which one is easier to read and maintain? Both seem equal to the task.
>
> I would prefer:
>
> my $count = $self->connection->notes('unrec_cmd_count') || 0;
> $self->connection->notes('unrec_cmd_count', $count + 1);
And now you too have been bitten by the strange and wonderful ways that notes
are evaluated in qpsmtpd. I made exactly your change:
https://github.com/msimerson/qpsmtpd/commit/c03a9156ea24a6b37f1f07ed120508314c67081a
And the incrementing test fails.
http://travis-ci.org/msimerson/qpsmtpd/builds/1531426
112
113# Failed test 'bad command 4'
114# at t/misc.t line 28.
115# got: '500'
116# expected: '521'
117# Looks like you failed 1 test of 14.
Try it yourself and see.
But I do like the idea of having $count increment without using the unusual
++$operator. So I changed it to this:
my $count = $self->connection->notes('unrec_cmd_count') || 0;
$count = $count + 1;
$self->connection->notes('unrec_cmd_count', $count);
And now it works again:
http://travis-ci.org/msimerson/qpsmtpd/builds/1531501
I'm pretty sure this "contrary to expectations" behavior of note value setting
is why a previous author of this plugin had the extra (unnecessary)
initialization method. I'm guessing it was test code, because he ran into
problems with the counter not incrementing reliably. Instead of writing tests,
he slung some extra code in there, including some that actually fixed it, and
the extra initialization method has been there ever since.
> Do you have a reason for the unconventional whitespace formatting?
It's called vertical alignment. It's a practice with roots in "The Elements of
Style", by Shrunk & White, circa 1918. It made a pass through 1974, it was
touched on by Kernighan's "Elements of Programming Style," where he advises
that we should "Format a program to help the reader understand it." But it
gets downright specific with Perl Best Practices, section 2.15, entitled
"Vertical Alignment", subtitled "Align corresponding items vertically."
It all boils down to, which of the following makes it more obvious to the
reader that both lines of code are related? Which can be comprehended faster?
my $count = $self->connection->notes('unrec_cmd_count') || 0;
$self->connection->notes('unrec_cmd_count', ++$count);
or
my $count = $self->connection->notes('unrec_cmd_count') || 0;
$self->connection->notes('unrec_cmd_count', ++$count);
Which one makes it easier to spot a typo? (and thus produces more reliable code)
There's also some fancy explanation for why the latter is easier to read, and
it involves our brains read ahead abilities, pattern recognition abilities, and
other cognitive mumbo jumbo that I don't remember much about. I don't bother to
(consciously) remember all that linguistics stuff. I just glance at them both
and see which one is easier to read.
Matt
