ANNOUNCE: Thesaurus - a recursive dictionary subclass using attributes

[Steven D'Aprano]

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