0

I have a class called User:

class User(object):

    def __init__(self, arg1, ..., traits = None):
        self.arg1 = arg1
        ...
        self.traits = traits

User has a property traits which is a dictionary. It's intended to be a catch-all that I can populate when I need to hold a bit more information about a user other than basic stuff like email address, userID, etc.

Would it be any more or less efficient to create a Traits class, and set the properties in it's __init__ with setattr? Would lookups work any better? Is there anything that this machinery would buy me, or is it just more work in the end?

BenDundee
  • 4,389
  • 3
  • 28
  • 34
  • 3
    re: `traits = {}` -- that's probably a bad idea. All `User` instances which don't have a `traits` dictionary passed will share the same instance. (See [here](http://stackoverflow.com/questions/1132941/least-astonishment-in-python-the-mutable-default-argument) for a fuller explanation.) – DSM Jan 24 '13 at 15:27
  • Nice...+1 for you. So `traits = None`, then. – BenDundee Jan 24 '13 at 15:29

2 Answers2

2

You can modify the instance's __dict__ directly (assuming that it doesn't implement __slots__ or anything silly like that)

Here's a simple example:

class Traits(object):
    pass

class User(object):
    def __init__(self,**traits):
        self.traits = Traits()
        self.traits.__dict__.update(traits)

a = User(eye_color="blue")
print a.traits.eye_color

Of course, it is probaby safer (but slightly less efficient) to just do:

class User(object):
    def __init__(self,**traits):
        self.traits = Traits()
        for k,v in traits.items():
            setattr(traits,k,v)

which is really only 1 additional line of code to provide the same API. It's safer because you never know what someone did to mess with Traits to provide various descriptors (e.g. property) etc. setattr will do all of that stuff correctly where blindly writing to __dict__ could be potentially harmful.

mgilson
  • 300,191
  • 65
  • 633
  • 696
  • You can, but you absolutely shouldn't. –  Jan 24 '13 at 15:26
  • @BenDundee: `for k, v in u.__dict__.items():`, for example. But I think keeping a traits dictionary is generally wiser. – David Robinson Jan 24 '13 at 15:27
  • @mgilson I wrote that when your post was just the first sentence. In trivial cases like your example (no other attributes), it's just an uglier way to use `getattr`/`setattr`, not actively harmful. But it sets a precedent for doing the same with other objects that might do "silly" things like providing any kind of descriptors, where using `__dict__` instead of `getattr`/`setattr` is plain wrong. You gain *nothing* by using `__dict__`, but potentially lose correctness. –  Jan 24 '13 at 15:31
  • @delnan -- You raise a valid point. I'm willing to bet that you gain performance using `__dict__`. Usually that's not a great reason to do something, but occasionally it is. – mgilson Jan 24 '13 at 15:34
2

Looks fine to me, I would be tempted to re-write as:

class User(object):
    def __init__(self, arg1, arg2, **traits):
        self.arg1 = arg1
        self.arg2 = arg2
        self.traits = traits

If you really wanted, you could then override to fall back on the traits dictionary if an attribute can't be found:

def __getattr__(self, name):
    return self.traits[name]

Example:

>>> u = User(1, 2, email='something')
>>> y = User(1, 2, email='bob', telno='999')
>>> u.arg1, u.arg2, u.email
(1, 2, 'something')
>>> y.arg1, y.arg2, y.email, y.telno
(1, 2, 'bob', '999')
>>> y.arg1, y.arg2, y.email, y.something
Traceback (most recent call last):
  File "<pyshell#105>", line 1, in <module>
    y.arg1, y.arg2, y.email, y.something
  File "/home/jon/adfafasdf.py", line 7, in __getattr__
    return self.traits[name]
KeyError: 'something'

So you may wish to make that a more sensible error, and change to:

def __getattr__(self, name):
    try:
        return self.traits[name]
    except KeyError as e:
        raise AttributeError() # with suitably formatted string
Jon Clements
  • 138,671
  • 33
  • 247
  • 280
  • Would you expect this to give any advantage memory-/readability-/performance-wise? – BenDundee Jan 24 '13 at 15:51
  • @BenDundee -- It's probably worse performance-wise, but it gives a slightly different API. `user.traits.whatever` becomes `user.whatever`. It gets a little interesting when you try to set attributes however ... You could get differences between `user.whatever` and `user.traits["whatever"]` depending on how you set attributes. Memory wise, It's probably slightly better too (classes take more memory than a dict), but really not a whole lot better. – mgilson Jan 24 '13 at 15:54
  • @mgilson Yup - I'm going from the view point that the basic stuff looks like it's attributes of the object, so keep traits as a dict, and fallback on unknown attributes and assume it's a possible trait. But yeah... we've come at the problem from two different ways - which is always a good thing! :) – Jon Clements Jan 24 '13 at 15:57