On Mon, 10 Dec 2012 22:48:50 -0500, Dave Cinege wrote:
> Thesaurus: A different way to call a dictionary.
Is this intended as a ready-for-production class?
> Thesaurus is a new a dictionary subclass which allows calling keys as if
> they are class attributes and will search through nested objects
> recursively when __getitem__ is called.
If only that were true...
py> d = Thesaurus()
py> d['spam'] = {}
py> d['spam']['ham'] = 'cheese' # key access works
py> d.spam.ham # but attribute access doesn't
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
AttributeError: 'dict' object has no attribute 'ham'
> You will notice that the code is disgusting simple. However I have found
> that this has completely changed the way I program in python.
By making it less robust and more prone to errors?
py> d = Thesaurus()
py> d.copy = "some value"
py> assert d.copy == "some value"
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
AssertionError
Operations which silently fail or do the wrong thing are not a good
thing. Scarily, this isn't even consistent:
py> "%(copy)s" % d
'some access'
py> d.copy
<built-in method copy of Thesaurus object at 0xb717a65c>
Some further comments about your code:
> class Thesaurus (dict):
> [...]
> def __getitem__(self, key):
> if '.' not in key:
> return dict.__getitem__(self, key)
> l = key.split('.')
> if isinstance(l[0], (dict, Thesaurus)):
> a = self.data
Testing for isinstance (dict, Thesaurus) is redundant, since Thesaurus is
a subclass of dict.
More importantly, there are no circumstances where l[0] will be a dict.
Since l is created by splitting a string, l[0] must be a string and this
clause is dead code.
Which is good, because self.data is not defined anywhere, so if you ever
did reach this branch, your code would fail.
> else:
> a = self
> for i in range(len(l)): # Walk keys recursivly
This is usually better written as:
for subkey in l:
# look up subkey
# or if all else fails
raise KeyError(subkey)
KeyError('spam.ham.cheese [1]') is misleading, since it implies to me
that looking up spam.ham.cheese succeeded, and the failed lookup was on
1. I would expect either of these:
KeyError('spam.ham')
KeyErroor('ham')
with a slight preference for the first.
Further issues with your code:
> try:
> a = a[l[i]] # Attempt key
> except:
Bare excepts like this are not good. At best they are lazy and sloppy,
good only for throw-away code. At worst, they are hide bugs. In this
case, they can hide bugs:
py> class X(object):
... @property
... def test(self):
... return 1/0 # oops a bug
...
py> d = Thesaurus(a=X())
py> d.a.test # Gives the correct exception for the error.
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "<stdin>", line 4, in test
ZeroDivisionError: float division
py>
py> "%(a.test)s" % d # Lies about the problem.
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "<stdin>", line 29, in __getitem__
KeyError: 'a.test [1]'
One more comment:
> # I like to create a master global object called 'g'.
> # No more 'global' statements for me.
> g = thes()
This is not a good thing. Encouraging the use of globals is a bad thing.
--
Steven
--
http://mail.python.org/mailman/listinfo/python-list
