Thread: [Postfixadmin-devel] About Trunk version

SourceForge Headquarters 225 Broadway Suite 1600 San Diego, CA 92101 +1 (858) 422-6466

Hello!

As I can see in trunk you try to rewrite PostfixAdmin with classes and
objects (OOP). It's very good and can really make PA more clear. But in fact
a code, which is present now in model/ folder, doesn't seem to be correct
OOP. To be clear, it's not an OOP at all. You've simply renamed

add_user($username, $quota, ....)

to

$user = new UserHandler($username);
$user->add($quota, ...);

There isn't any preferences in such renaming. The main goal in OOP is that
one class (object) represent a *one* entity. For example, class User
represent one user and *no more*. But now, e.g. UserHandler::add, try to
represent whole PostfixAdmin, because it do (or will do, when you move code
from old create-user.php) auth and privilege checks, domain checks and so
on. But if it's a user, it shouldn't do anything, exept a work with one
user. AliasHandler has same problems.

I understand, that you haven't got enough time to develop PA, but, may be,
it's not a good idea to do extra work and write a lot of useless code.

Below some concept of possible architecture for PA based on OOP:

There is one main class (e.g. pa_main), which handle sessions,
configuration, language options etc. It has static field - $instance, which
store an object of this class, and static method - getInstance(), which
return value of $instance. It can be used like:

  pa_main::getInstance()->getText($name);
  pa_main::getInstance()->getConf($param);

There is only one class to handle all operations with users. E.g. pa_user.
pa_main has a filed $user, which store an pa_user object, which represent
current logged in user.

pa_user can have methods like: changePasswd($newpass), checkPasswd($pass),
isAdmin(), isDomainAdmin($domain), save() [save new pa_user to database],
update() [update userinfo in database from current object]. Object of
pa_user should store all data for this user from mailbox table of database.

pa_user shouldn't do any checks, which are not associated with the db table
mailbox. And other parts of PA shouldn't interact with mailbox table
directly, only by creating pa_user objects.

Similar scheme should be for aliases and domains. And you can also add
pa_userslist and pa_aliaseslist to work with groups of users and aliases
(e.g. for search).

Next: there should be database handler class, e.g. pa_db. pa_main should
store one object, which is represent db. E.g.

pa_main::getInstance()->getDbh()->query('...');

The similar concept is using in RoundCube - you can look to it's source
code. And it's easy to work with RC, because each part of it do their
specific job.

And you should merge two login pages (for admins and for users) into one and
delete two tables - admins and domain_admin, because they repeat the table
mailbox and are totally redundant. There shouldn't be also two main pages
etc. In fact, if you switch to
architecture, based on object and classes, all duplicate pages will be
removed automatically.

I hope, you'll get something usefull from my letter and will develop PA to
be more clear. And will not make classes like UserHandler :)

Best regards, Vadim Nevorotin.

Thread: [Postfixadmin-devel] About Trunk version

postfixadmin-devel