Code review comment for lp:~mbp/launchpad/612171-diff-generation-spam

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 11-10-30 08:49 PM, Martin Pool wrote:
>> Was providing retry_error_types as a list rather than a tuple
>> deliberate? Given that the main difference between lists and
>> tuples is that lists are mutable, are you intending the class
>> variable to be mutated? Since that would affect all instances of
>> the class, I think that could be confusing.
>
> It was intentional. I think there is a semantic difference which
> is that lists convey there can be any arbitrary number of elements
> and all the elements are treated identically, which is the case
> here. I think tuples are generally more important when the
> structure is more like a mathematical vector with different
> positions having different meanings or dimensions.

The meaning you give to tuples is not consistent with the way tuples
are used in except clauses. Except clauses use a tuple to specify a
sequence of fixed but arbitrary length, in which all elements are
treated the same. The retry_error_types member is that portion of an
except clause, extracted to a member variable. isinstance has the
same semantics. So I think a tuple conforms to Python style, and a
list varies from it.

> I don't feel that getting protection against "accidental" mutation
> is important here because there are so many other ways that can be
> broken in Python, most obviously by just assigning to the attribute
> on either the instance or the class.

Python usually assumes that we're consenting adults, so there's very
little you can't do, if you try.

There's a big difference between "self.retry_error_types.append" and
"self.retry_error_types = self.retry_error_types +". The second will
affect only the instance, while the first will affect all instances.
You can affect all instances, if you want, but you have to spell that out.

> But, I don't really care, if Launchpad style or consistency wants a
> tuple I'll change it.

Please do, and consider that it's consistency with Python, as well as
with Launchpad.

Aaron
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAk6uE2wACgkQ0F+nu1YWqI36MQCfelBM65Tl8SU6HrKro5+8tnXs
tVAAniGZHw1CpL2eA+U8wjaSLr0RFYCE
=ag6f
-----END PGP SIGNATURE-----