David Goodger wrote:
> When I first created the docutils.nodes.Element class, I gave it a
> hybrid indexing API: list (for children) + dict (for attributes). In
> other words, element[0] returns the first child, and element['id']
> returns the "id" attribute.
>
> This is a convenient API, but often "considered harmful".
It's probably not entirely clean design (in the academic sense of
"clean"), but it's quite convenient. Attribute access is done so often,
and node['classes'] is just much terser than node.attributes['classes'].
> I didn't really see why until Fred Drake (at PyCon 2004 IIRC) gave an
> apt example:
>
> Q: If ``i`` is a variable, what does ``element[i]`` return?
>
> A: It depends on the type of ``i``. If ``i`` is a parameter, we
> don't really know without explicit testing, and this could hide
> bugs.
A: ``i`` is used as an integer variable in for-loops by convention, so
element[i] returns the i'th child. :-)
Quite seriously, I've never found it to be ambiguous in practice. Most
times attributes are accessed it's done using a literal string anyway.
> Since my enlightening, I've thought about changing the API to use
> __(get|set|del)item__ for list indexing only, and change node
> attributes access to explicitly use ``Element.attribute``.
IMO Element.attributes['...'] is much too long. I'd by OK with
Element.atts['...'] (because it's shorter), but I don't really see
Element['...'] as a problem.
In any case, I think attribute access should be done consistently
throughout the Docutils code.
> The existing API is implemented by examining the type of the index
> with a bunch of isinstance() calls, in
> docutils.nodes.Element.__getitem__, .__setitem__, and __delitem__. If
> we remove node attribute indexing, we could get rid of all those
> isinstance calls and simplify the code considerably.
Yeah, but I think the convenience is worth the complicatedness in the
implementation. And after all, that code is seldom touched, and the
profiler doesn't show any performance issues either.
So I suggest we leave it as is.
--
For private mail please ensure that the header contains 'Felix Wiemann'.
http://www.ososo.de/
|