views:

156

answers:

5

I'm trying to figure out the best way to design a couple of classes. I'm pretty new to Python (and OOP in general) and just want to make sure that I'm doing this right. I have two classes: "Users" and "User".

class User(object):
   def __init__(self):
       pass

class Users(object):
   def __init__(self):
       self.users = []

   def add(self, user_id, email):
       u = User()
       u.user_id = user_id
       u.email = email
       self.users.append(u)

users = Users()
users.add(user_id = 1, email = '[email protected]')

If I want to retrieve my users, I use:

for u in users.users:
    print u.email

"users.users" seems to be a bit redundant. Am I doing this right?

+10  A: 

Put this in your Users:

def __iter__(self):
    return iter(self.users)

Now you can:

for u in users:
    print u.email

Docs

recursive
+6  A: 

You probably just want a list of User objects rather than a class which contains multiple users.

class User(object):
   def __init__(self, user_id, email):
       self.user_id = user_id
       self.email = email

users = []
users.append(User(user_id = 1, email = '[email protected]'))

All member attributes for User should reside in the User class, not the Users class.

cquillen
+4  A: 

I don't see anything wrong with users.users, but if you prefer a nicer way to do it you could override __iter__ in Users.

class Users(object):
   def __init__(self):
       self.users = []

   def add(self, user_id, email):
       u = User()
       u.user_id = user_id
       u.email = email
       self.users.append(u)

   def __iter__(self):
       return iter(self.users)

Now you can do this:

for u in users:
    print u.email

The __iter__ special method makes your object behave as an iterator

Nadia Alramli
+17  A: 

I would say not really. Your Users class appears to just be a list of users, so I would just make it a list rather than a whole class. Here's what I would do:

class User(object):
    def __init__(self, user_id=None, email=None):
        self.user_id, self.email = user_id, email

users = []
users.append(User(user_id = 1, email = '[email protected]'))

for u in users:
    print u.email

If you want Users to be a class of its own for some other reason, you could have it inherit from list, or (if not) you could add these to the definition:

class Users(object):
    # rest of code
    def __iter__(self):
        return iter(self.users)

That way, you can simply say:

users = Users()
...
for u in users:
    print u.email
Chris Lutz
TypeError: iteration over non-sequence -- if you're using += the RHS should also be a list.
Jorenko
+1  A: 

There's no "black" and "white" here, just shades of grey. You don't need a special Users class if it's just going to be a list.

Another way:

class User:
    all_users = []

    def __init__(self, id, email):
        self.id = id # No need to call it user_id - it's a User object, after all!
        self.email = email
        self.all_users.append(self) #automatically add to list of all users

    def __str__(self):
        return '%s(%s)' % (self.id, self.email)

Then, if you typed the above into user.py:

>>> from user import *
>>> bob = User('bob', '[email protected]')
>>> alice = User('alice', '[email protected]')
>>> for u in User.all_users:
...     print u
...
bob([email protected])
alice([email protected])
>>>

Just an example to get you thinking.

Vinay Sajip
Shouldn't it be `self.all_users.append(self)` or even `A.all_users.append(self)` (both work) instead of `self.users.append(self)`?
voyager
Yes, typo now corrected. Thanks for the tip-off!
Vinay Sajip