[ postfixadmin-Feature Requests-3006020 ] Advanced Config and Lang

[SourceForge.net <no...@so...>]

Feature Requests item #3006020, was opened at 2010-05-23 16:53
Message generated for change (Comment added) made by christian_boltz
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=937967&aid=3006020&group_id=191583

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: Core
Group: None
Status: Open
Resolution: None
Priority: 5
Private: No
Submitted By: Valkum (valkum)
Assigned to: Nobody/Anonymous (nobody)
Summary: Advanced Config and Lang

Initial Comment:
I think about use an extended version of Config and Lang variables.
In my CLI I use a simple class, cloned from Joomla!.
I think this should be implemented in functions.inc.php.
With this class you can read even lang and conf option in classes. This is not working with globals.

----------------------------------------------------------------------

>Comment By: Christian Boltz (christian_boltz)
Date: 2011-10-27 22:01

Message:
Well... ;-)

gingerdog wrote:
> Changing the configuration to be in an object which is a singleton and
> silently used everywhere isn't really any different to using a global
array
> - it's still "a magic thing" which the callee/user has no direct
control
> over. The code has a high coupling to Config (the class) just in the
same
> way as it had a high coupling to the $CONF array.

We won't get dependency-free code, and I don't see a problem with
depending on the Config class. (And I don't think it's a problem that the
caller/user has no direct control over it.)

As a general note: My reason for migrating functionality to the *Handler
classes is not to make the code object-orientated, MVC-compliant,
encapsulated etc.

The real reason for me is to have _easy to maintain_ code. And that's
something where classes are great - we can move the code that is common for
all *Handler classes to PFAHandler and it's available in all *Handler
classes automatically. And we still have the flexibility to override some
parts by (re)defining them in each *Handler class. The AliasHandler will
probably only need the functions initStruct and initMsg, MailboxHandler
will need some more (send welcome mail etc.).

If we get object-orientated, MVC-compliant, encapsulated etc. code as a
side effect: Nice, why not. 
If I have to ignore some holy grail rule of object-orientated programming
to reach the target "easy to maintain code", then I'll just ignore the rule
;-)

I can already see you screaming when I implement hooks for
editing/customizing the $struct array ;-)) but nevertheless those hooks
might be one of the most important improvements in 3.0 because they make it
easy to add custom fields, hide existing fields etc.

> Really the model classes should support injection of the Config/Lang
> objects so they can be (if necessary) swapped for something else to aid
> testing and reuse.

You can easily replace the content of Config/Lang using
Config::write/Lang::write, so I don't see a real problem here.

> If Lang has no functionality over/above a Config object, is it worth
> having having - why not just store the stuff in Config?

One comment earlier you complained about "not really clean" code ;-) 
From the technical side we don't need a separate object for the texts and
could store them in Config - but texts are not config options. I'd say we
keep separate objects if they don't cause any problems (I have a server
with PHP 5.2.14 and will test the behaviour there when I find some time.)


valkum wrote:
> What about changing the whole Lang object to use a helper function
> t("string")

That doesn't change much (except saving some bytes), but makes the code
harder to understand. I prefer human-readable function/class calls. _If_ we
decide to use the Config object for the texts, using something like
Config::Lang('string') might be the best solution.

----------------------------------------------------------------------

Comment By: Valkum (valkum)
Date: 2011-10-27 01:55

Message:
What about changing the whole Lang object to use a helper function
t("string")

t("string") looks in Conf for language and include_once the matching
language file when it is not already included. then t() returns the
matching string


----------------------------------------------------------------------

Comment By: GingerDog (gingerdog)
Date: 2011-10-26 01:14

Message:
(I'm also not sure how the Lang class will actually behave, especially
under PHP5.2 where there isn't late static binding. I have a feeling
calling Lang::getInstance() may actually return a Config object.

If Lang has no functionality over/above a Config object, is it worth
having having  - why not just store the stuff in Config?

----------------------------------------------------------------------

Comment By: GingerDog (gingerdog)
Date: 2011-10-26 01:12

Message:
<2p>
To be argumentative - 

Changing the configuration to be in an object which is a singleton and
silently used everywhere isn't really any different to using a global array
- it's still "a magic thing" which the callee/user has no direct control
over. The code has a high coupling to Config (the class) just in the same
way as it had a high coupling to the $CONF array.


Really the model classes should support injection of the Config/Lang
objects so they can be (if necessary) swapped for something else to aid
testing and reuse.
</2p>

----------------------------------------------------------------------

Comment By: Christian Boltz (christian_boltz)
Date: 2011-10-26 01:02

Message:
In the meantime I moved your classes to model/ and removed duplicate code -
the "Lang" class shrunk to 

class Lang extends Config {
    # exactly the same code, just another name ;-)
}

;-)

Some functions already use Lang::read and Config::read, the others will
follow sooner or later.

----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=937967&aid=3006020&group_id=191583