[Zope-dev] TreeVocabulary in zope.schema.vocabulary

[Charlie Clark]

Hiya,

Am 26.01.2012, 15:02 Uhr, schrieb Jan-Carel Brand <lists at opkode.com>:

> Ok, Charlie also expressed his reservations. I'll put it in a different
> package then.

Hang on a minute! While I'm not 100 % convinced of the need in the core I  
think a separate package just for TreeVocabulary would be splitting hairs.  
If z3c.form can use it then I think that is justification enough.

> I'm not too sure what to name it though. For example, under what
> namespace? zope or z3c?
> I'm guessing zope.vocabulary, or rather zope.treevocabulary?
>
>> Furthermore, for the dict class in use in the vocabulary, you could
>> add a "factory" class that can be overriden easily.
>> That would allow people with OrderDict capabilities to use them
>> without having to re-sort later on.
> Could you please elaborate on what you mean?
> If I create a factory class to create TreeVocabulary instances, how will
> overriding that factory (without creating a separate
> SortableTreeVocabulary) allow people to use OrderedDict?
> Incidentally, I came upon this: http://pypi.python.org/pypi/ordereddict
> which provides the OrderedDict to Python 2.4 to 2.7

> I think it might make sense to just subclass OrderedDict and implement
> an ordered tree from the start.

I agree. Despite my previous remark about class methods, I don't think we  
need to worry much about Python 2.4 and 2.5 and ordered dictionaries are  
just so damn useful that they've been added to the standard library.

Back to bike-shedding. As I was intrigued by the whole thing I've spent  
some time looking at the code. I'm not too happy on the use of nested  
functions as I find they obscure code, particularly when used recursively.  
I think that "createTree" and "recurse" should be written as separately  
testable generators. I also don't see a need for createTerm in this  
particular class and the subsequent awkward call from createTree. As it  
stands it is copy & paste both in method and test. If you must have it  
with the same implementation

createTree = SimpleVocabulary.createTree

does the job just fine but I don't see the advantage of  
cls.createTerm(*args) over SimpleTerm(*args)

As I said this is bike-shedding but I think our source code should be  
written with a view to being read and probably copied verbatim. With that  
in mind I prefer readability and testability over integration. In the end  
it tends to make things easier to use. The exceptions where refactoring to  
produce slightly uglier code but with significant performance hopefully  
prove the rule.

Charlie
-- 
Charlie Clark
Managing Director
Clark Consulting & Research
German Office
Kronenstr. 27a
Düsseldorf
D- 40217
Tel: +49-211-600-3657
Mobile: +49-178-782-6226