[PATCH 0 of 2 RFC] Introduce defaultdict in mercurial?
Nicolas Dumazet
nicdumz at gmail.com
Fri Aug 21 10:43:10 CDT 2009
Hello -devel!
I find defaultdict, the Python 2.5 feature, very useful and quite elegant in
many cases. I am trying today to introduce this feature in mercurial.
Since we still support Python 2.4, I introduce in util a simple wrapper around
dict to emulate defaultdict for Python 2.4 users. >= 2.5 users have a native,
C implementation of defaultdicts.
Any thoughts about this?
In our codebase we have sometimes cases where setdefault is used at
initialization:
for p in ...:
children.setdefault(p, []).append(n)
But then, later, for some reason we still have to use get() because not
every key seems safe:
for c in children.get(n, []):
foo()
I think that in those case in particular, it's cleaner and simpler to
declare children as a defaultdict(list):
children = util.defaultdict(list)
# ...
for p in ...:
children[p].append(n)
# ...
for c in children[n]:
foo()
Do you agree too that in some cases it is helpful and cleaner to use
defaultdict?
The second patch attempts to replace usage of dict+setdefault by a defaultdict,
where relevant. Do you agree with my choices?
I left alone some cases. For example, in locarepo.py:
# Create the set of filenodes for the file if
# there isn't one already.
ndset = msng_filenode_set.setdefault(f, {})
# And set the filenode's changelog node to the
# manifest's if it hasn't been set already.
ndset.setdefault(fnode, clnode)
The two lines could be changed to msng_filenode_set[f].setdefault(fnode, clnode)
but at the sake of readability. I'd rather have longer and more readable code
where needed.
(As a side question: what guidelines are there for submitting rather long
refactoring patches? I understand that the second patch is probably quite long
and hard to review; does it makes sense to split the commit per filegroups,
maybe?)
Thanks,
Nicolas.
More information about the Mercurial-devel
mailing list