On Tue, Dec 11, 2012 at 1:57 PM, Dave Cinege <d...@linkscape.net> wrote: > On Tuesday 11 December 2012 01:41:38 Ian Kelly wrote: >> Second, in __getitem__ you start a loop with "for i in >> range(len(l)):", and then you use i as an index into l several times. >> It would be cleaner and more Pythonic to do "for i, part in >> enumerate(l):", and then you can replace every occurrence of "l[i]" > > My python is still 'old school' due to being stuck on old versions for in > production embedded system python applications.
Just out of curiosity, how old are we talking? enumerate was added in Python 2.3, which is nearly 10 years old. Prior to 2.2 I don't think it was even possible to subclass dict, which would make your Thesaurus implementation unusable, so are these systems running Python 2.2? >> It's not clear to me what the isinstance call here is meant to be >> testing for. > > It's used to determine if it's the root instance of the recursive string > because self.data, not self must be used to access that. Can you offer a > better > way? > >> The prior statements require key to be a string. If key >> is a string, then by construction l[0] is also a string. So it seems >> to me that the isinstance check here will always be False. > > OK, try and remove it and then get back to me. :-) Okay. I replaced this code: if isinstance(l[0], (dict, Thesaurus)): a = self.data else: a = self with: a = self and then I ran the examples, and the output was unchanged. As Steven pointed out, I don't see how that first branch could succeed anyway, since self.data is never defined. -- http://mail.python.org/mailman/listinfo/python-list