Re: [Postfixadmin-devel] About Trunk version

[Christian B. <pos...@cb...>]

Hello,

(CC'ing you because you are not subscribed to postfixadmin-devel)

Am Montag, 28. Februar 2011 schrieb Неворотин Вадим:
> 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, ...);
[...]
> 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.

I have to disagree slightly ;-)

You are correct that model/* are not 100% pure OOP.

I'd like to tell you our current targets regarding those files:

First and most important: 
have one instance of the code for doing a job - and not one in "create-
mailbox", one in "edit-mailbox" and another one in the commandline 
client.

Besides that, I want to have a common interface in all classes (where it 
makes sense - things like "create", "update", "delete"), so that 
- adding custom fields is very easy (see $fm_struct and $fm_defaults in 
  fetchmail.php - it will become something like this)
- creating a new module in PostfixAdmin is easy (I'm dreaming of 
  something like "define two arrays, add a menu entry and you'll get 
  list and edit mode including input validation. Yes, that is possible.)

Of course I also have the "usual" reasons for OOP in mind, but not 
always as primary goals.

> Below some concept of possible architecture for PA based on OOP:
[...]

That looks like 100% OOP.

Don't get me wrong - there's nothing wrong with OOP, but I don't insist 
on using OOP just because it is OOP. There has to be a clear advantage.

For example, I don't see the advantage of a DB class over the db_* 
functions we have, and therefore I'm not too keen to write 
    pa_main::getInstance()->getDbh()->query('...');
instead of
    db_query('...');

(I hate XSLT for similar reasons ;-)

OTOH, your mail also contains lots of useful ideas for the model/* 
files. I won't comment on each of them and just say "thanks!" instead.

> And you should merge two login pages (for admins and for users) into
> one 

That's not as easy as it sounds - what if there are an admin and a 
mailbox with the same name? Choose the one where the password matches? 
;-)

> and delete two tables - admins and domain_admin, because they
> repeat the table mailbox and are totally redundant. 

They are not - if you check the content of those tables, you'll notice 
that they contain very different stuff (especially domain_admin).

If your intention is to use mailbox passwords for admin logins: that's 
something we discussed some time ago, but I don't really like it.

> 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.

You don't even want to know how much duplicate code I removed since I'm 
working on PostfixAdmin ;-) For example, edit-alias existed for domain 
admins, for superadmins and for users - and each of them had nearly the 
same code (except the UPDATE query) duplicated for GET and POST...
(If you are really interested, checkout SVN r1 from sourceforge and look 
at the code. But: you have been warned! *eg*)

There's no need to tell me that duplicate code is evil - the problem is 
that it needs some development time to remove/merge it (usually every 
copy comes with different bugs ;-) ), and unfortunately this doesn't 
happen "automatically".

So we have the choice between 
a) doing a "90% OOP" class quite fast to avoid that we have to maintain  
   3 copies of the same functionality - or -
b) do 100% OOP with at least double development time (you'll always find 
   something that is not OOP'ed yet) and have to maintain the old code 
   until that is done.

My choice for now is "90% OOP", and the reason is quite easy: with the 
same time spent, we'll only have 50% of the "100% OOP" done, and still 
have to manage half of the "old" code ;-)

The long term handling might be different and tend to 99% OOP ;-)

> 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 :)

The layout and interface is not set in stone (and not perfect). But it 
is much better than what we had before, and I already have some things 
in mind that I want to change. Unfortunately, the same problem about 
"time" applies here.

Nevertheless: thanks for your comments!


Regards,

Christian Boltz

PS: hand-picked .sig today ;-)
-- 
In most cases, XSLT is good enough. But I agree, for some parts
you need Aspirin. ;-)        [Thomas Schraitle in opensuse-doc]