views:

129

answers:

4

(the title is admittedly not that great. Please forgive my English, this is the best I could think of)

I'm writing a python script that will manage email domains and their accounts, and I'm also a newby at OOP design. My two (related?) issues are:

  1. the Domain class must do special work to add and remove accounts, like adding/removing them to the underlying implementation
  2. how to manage operations on accounts that must go through their container

To solve the former issue I'd add a factory method to the Domain class that'll build an Account instance in that domain, and a 'remove' (anti-factory?) method to handle deletions.

For the latter this seems to me "anti-oop" since what would logically be an operation on an Account (eg, change password) must always reference the containing Domain. Seems to me that I must add to the Account a reference back to the Domain and use that to get data (like the domain name) or call methods on the Domain class.

Code example (element uses data from the container) that manages an underlying Vpopmail system:

class Account:
    def __init__(self, name, password, domain):
        self.name = name
        self.password = password
        self.domain = domain
    def set_password(self, password):
        os.system('vpasswd %s@%s %s' % (self.name, self.domain.name, password)
        self.password = password

class Domain:
    def __init__(self, domain_name):
        self.name = domain_name
        self.accounts = {}
    def create_account(self, name, password):
        os.system('vadduser %s@%s %s' % (name, self.name, password))
        account = Account(name, password, self)
        self.accounts[name] = account
    def delete_account(self, name):
        os.system('vdeluser %s@%s' % (name, self.name))
        del self.accounts[name]

another option would be for Account.set_password to call a Domain method that would do the actual work - sounds equally ugly to me.

Also note the duplication of data (account name also as dict key), it sounds logical (account names are "primary key" inside a domain) but accounts need to know their own name.

EDIT: please note the above code is just a quick example, think of it as pseudo code. It intentionally does not care about error conditions or security issues, and is incomplete in data and methods of the classes (per-user spam settings, auto-responders, forwarders, get mailbox size, etc...).

Also, this is an example I had at hand, but I think it could be generalized to other different logical structures similar to trees where nodes must know about their children and children must call into parents (or upper level ancestors) to do things. To me sounds logically similar to class inheritance but applied to instances of different types (classes) linked to each other.

+1  A: 

I think that you don't need the methods create/delete account in the Domain class. I would rather have it like this:

class Account:
    def __init__(self, name, password, domain):
        ...

    def activate(self):
        self.domain.add(self)
        os.system('vadduser %s@%s %s' % (name, self.domain.name, password))

    def deactivate(self):
        self.domain.remove(self)
        os.system('vdeluser %s@%s' % (name, self.domain.name)

If you have many such relations between objects, I believe the standard option is to use a database. One of the most popular for python is SQLAlchemy. It will solve the problem of efficiently storing relationship and looking them up (and much more). But in your example, it's obviously an overkill, and I suppose the only option is to handle that manually as in my code.

Olivier
You usually ask questions like: "how many accounts does this domain have?" or "is there an account foo in domain bar?" - the entry point is nearly always the domain and you want to do CRUD operations on accounts contained there.While I can decouple the class design from the system interface, and just 'ask' the domain for a list of accounts without actually having python objects for them, I thought this pattern could be common (every time you have some tree-like structure with relations between parent and child nodes) and was looking for a generic solution - mail was just an example.
Luke404
Well, you were asking for advice in object-oriented programming, so I answered that. The activation of an account is definitely a method of the `Account` class. As to the relationships, I edited my answer accordingly.
Olivier
+2  A: 

For the operations you've outlined, it's not clear that you need Account at all. The only information it holds that is not already duplicated in Domain is the password. You could just have Domain.accounts being a lookup of username: password instead.

Don't multiply identity-bearing classes until you need to.

For what it's worth in the general case, yes, when you have objects that are owned by other objects it's quite normal to give them a reference up to their owner and have them communicate upwards as needed. Python doesn't have the notion of inner classes that some languages provide for ownership.

(Incidentally, don't concatenate strings into command lines for os.system; this is a serious security risk. See the subprocess module for a safer and easier way to pass parameters.)

bobince
While not directly related to this question, thanks for the os.system tip, I'll look into that whenever I'll have to call external binaries.
Luke404
+1  A: 

In Python one tends to avoid doing recursive relations, because the garbage collector is usually implemented as a reference counting scheme.

Simplest solution: do the operations that need the container in the container.

A little bit more convoluted solution: when the container is queried for an object, create a temporary proxy object that holds a reference to both the container and the contained object and implements the interface of the contained; and return it instead of the contained object.

fortran
Arrrg, that's wrong.Circular relationships get collected by the circular relationship garbage collector. One doesn't "tend to avoid doing that" in Python more than any other langage...
moshez
Maybe I explained myself wrong, what I wanted to say is that the cyclic reference detection is built optionally (at least I think I saw a flag to enable or disable it when making Python from sources), so I'd better not relay on that if I can avoid it without extra effort.
fortran
Circular references will usually get collected by the GC - just not as fast as standard references. But you need to pay attention because the GC will never clean up any instance with circular references AND a user defined __del__() method.At least, this is the current state of the CPython implementation - these details are implementation-specific.
Luke404
@Luke404 thanks for the extra information :-)
fortran
A: 

So, you have domains, and you have accounts, and the real work of your application is managing accounts, including their associations with domains...

Why don't you just create a Python "Manager" or "AccountManager" class that can be a puppetmaster for the Domains and the Accounts? It'll remove questions like the one you've posted altogether by introducing an 'objective third party' who can reference any of the other objects and make associations between them at will.

class Manager(object):
    def set_password(self, domain, account, password):
        os.system('vpasswd %s@%s %s' % (account.name, domain.name, password)
        account.password = password

>>> m = Manager()
>>> d = Domain('google.com')
>>> a = Account('foouser')
>>> m.set_password(d, a, p)

Of course, in a real program, you'd create one manager object and then act upon as many domains and accounts as you want with that one instance.

jonesy