launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Thanks for the suggestion.  I've thought about it and I think I still prefer the lambda form here.  My reasoning inasmuch as I can articulate it:

 1) The cited reason to prefer other forms over lambda is that naming functions makes them easier to debug.  While that's fair in general, I have difficulty seeing it coming into play here: the return value is slightly interesting, but not particularly the execution of the function, and the former is very straightforwardly visible from the caller anyway.

 2) I tend to feel that a useful barometer for this kind of thing is to consider adding additional levels.  Before I noticed that the suite was redundant, I had a three-level defaultdict instead, so it was "overrides = defaultdict(lambda: defaultdict(lambda: defaultdict(set)))".  Now, one might argue about whether this is readable enough (I think that simplifying the dictionary population code is worth it, but it's certainly up for debate).

    The alternative would be "overrides = defaultdict(defaultdict(defaultdict(set).copy).copy)".  Admittedly that's a bit more symmetrical, but I find it harder to follow; constructs where you just close all the parentheses at the end are easier to parse visually because you can see at a glance that you don't need to keep as careful track of how many levels deep you are.  Also, I had to test this to reassure myself that it actually did the right thing rather than copying a partially-populated defaultdict, whereas the correctness of the lambda form is clear once you have the structure.

 3) (weak) There's precedent for the lambda form:

   lib/lp/bugs/model/structuralsubscription.py:793:    statuses = defaultdict(lambda: defaultdict(list))
-- 
https://code.launchpad.net/~cjwatson/launchpad/unnecessary-publisher-work/+merge/120356
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.