Instance variable: future direction - function, variable, and inject dependency?

[garyamort]

Speaking of static variables, I am a bit confused on the instance variable.

In *most* cases, the instance variable is a static variable defined in the getInstance function...making it difficult to override or change in any way[I think some complex work with the reflection class might be able to update it].

However, in the *new* code in the system, it tends to be a static variable for the class.  See JForm, JFilterInput, JDatabase, JLanguage, and JURI

So the question I have is if these classes are exceptions to the standard, or if these classes are the new standard?

My own preference is to store the instances outside of the function as then it is much easier to get to and override that code[If you subclass a class with a static variable, your subclass shares the parent classes public static variables.  However, you cannot override static variables stored in functions - that creates a new static variable specific for the subclass].

I also imaging that these instance variables cause difficulties with unit testing.  Adding the ability to pass an object to getInstance  to specify what instance you want to be used makes it easier to create mock objects for testing..

Basically, this is the only direct change I had to make to JMail for my own personal website as I heavily customized my JMail instance to allow it to send notices through Mailchimp, Amazon Web Services email platform, RightScale, and my own custom solution.  Basically I wanted open and click tracking, so pumping the notices through those services gives me that ability.  My class is a subclass of JMail and then I just override what I want to change and add a system plugin to create my custom instance and override JMail's.   If JMail had stored the $instance variable as a global, it would have been easier since then I could have accessed it directly and overridden it without having to hack core.

My change:

/**

* Returns the global email object, only creating it

* if it doesn't already exist.

*

* NOTE: If you need an instance to use that does not have the global configuration

* values, use an id string that is not 'Joomla'.

*

* @param   string  $id  The id string for the JMail instance [optional]

 * @param   string  $instance  An instance of a  JMail object to be set as the new instance[optional]

*

* @return  JMail  The global JMail object

*

* @since   11.1

*/

public static function getInstance($id = 'Joomla', $instance = null)

{

static $instances;

if (!isset($instances))

{

$instances = array();

}

            if (is_a($instance, 'JMail'))

{

$instances[$id] = $instance;

}

  

if (empty($instances[$id]))

{

$instances[$id] = new JMail();

}

return $instances[$id];

}

[Niels Braczek]

On 02.09.2011 16:24, garyamort wrote:

> In *most* cases, the instance variable is a static variable defined in the
> getInstance function...making it difficult to override or change in any
> way[I think some complex work with the reflection class might be able to
> update it].

That is legacy PHP4 style.

> However, in the *new* code in the system, it tends to be a static variable
> for the class. See JForm, JFilterInput, JDatabase, JLanguage, and JURI

That's the way it should be in PHP5+ (although singletons are evil
regarding testability).

> So the question I have is if these classes are exceptions to the standard,
> or if these classes are the new standard?

The latter.

> My class is a subclass of JMail and then I just override what I
> want to change and add a system plugin to create my custom instance and
> override JMail's. If JMail had stored the $instance variable as a global,
> it would have been easier since then I could have accessed it directly and
> overridden it without having to hack core.

With a correct design, the Mailer instance would not depend on the name
"JMail", but would be created by the main controller (or a plugin to
it), and be injected into a dependency container for further use.
Unfortunately, a lot of the techniques used, are heritage from old Mambo
times and have not been corrected until now. Let's hope, this will be
done in future versions (there are a lot of signs showing this will happen).

Until then, don't derivate from JMail, but make a copy of it and change
it directly. Then register your class with JLoader at system start
(using a system plugin). Then the first call to JMail will pull *your*
class instead of the original - without changing the core.

HTH
Niels

--
| http://www.kolleg.de · Das Portal der Kollegs in Deutschland |
| http://www.bsds.de · BSDS Braczek Software- und DatenSysteme |
| Webdesign · Webhosting · e-Commerce · Joomla! Content Management |
------------------------------------------------------------------

[Louis Landry]

Niels' response echo's nearly everything I would have said.

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

Louis Landry

louis....@gmail.com

--
You received this message because you are subscribed to the Google Groups "Joomla! Framework Development" group.
To post to this group, send an email to joomla-dev...@googlegroups.com.
To unsubscribe from this group, send email to joomla-dev-frame...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/joomla-dev-framework?hl=en-GB.

[garyamort]

On Saturday, September 3, 2011 7:40:08 PM UTC-4, Niels Braczek wrote:

On 02.09.2011 16:24, garyamort wrote:

The latter.

> My class is a subclass of JMail and then I just override what I
> want to change and add a system plugin to create my custom instance and
> override JMail's. If JMail had stored the $instance variable as a global,
> it would have been easier since then I could have accessed it directly and
> overridden it without having to hack core.

With a correct design, the Mailer instance would not depend on the name
"JMail", but would be created by the main controller (or a plugin to
it), and be injected into a dependency container for further use.

Unfortunately, a lot of the techniques used, are heritage from old Mambo
times and have not been corrected until now. Let's hope, this will be
done in future versions (there are a lot of signs showing this will happen).

Until then, don't derivate from JMail, but make a copy of it and change
it directly. Then register your class with JLoader at system start
(using a system plugin). Then the first call to JMail will pull *your*
class instead of the original - without changing the core.

I still see that as hacking core, it's just keeping your code out of the core library folder.

What I do is make a minimally changed copy of the core file to allow me to override the getInstance method, then I create my own custom subclass.

So in this case, I have JMail and GMessage  My custom JMail class is loaded in a system plugin, followed by loading GMessage, I then create a new instance of GMessage, $gmessage, and then pass that to a static call to JMail::getinstance('joomla', $gmessage); 

That allows me to keep all my customization exclusive to the GMessage class, while still allowing it to 'just work' for other components which expect to use JMail.  

The only change I see to make here is knowing that the future direction is to move the static variable from the function to the class I'll include that in my change as well.

-Gary

[Niels Braczek]

On 04.09.2011 17:40, garyamort wrote:
> On Saturday, September 3, 2011 7:40:08 PM UTC-4, Niels Braczek wrote:

>> Until then, don't derivate from JMail, but make a copy of it and change
>> it directly. Then register your class with JLoader at system start
>> (using a system plugin). Then the first call to JMail will pull *your*
>> class instead of the original - without changing the core.

> I still see that as hacking core, it's just keeping your code out of the
> core library folder.

Then you have a wrong perspective. The main advantage of my solution
over yours is, that it will survive updates to the core, because it is
*not* a core hack. It uses the way, Joomla! offers to override core
classes. As said, it is not the best technique, but it works well.

> What I do is make a minimally changed copy of the core file to allow me to
> override the getInstance method, then I create my own custom subclass.

But on any update, your change gets lost, because it is a core hack.

> That allows me to keep all my customization exclusive to the GMessage class,
> while still allowing it to 'just work' for other components which expect to
> use JMail.

JMail offers the possibilty to have different instances for different
purposes. Your solution is ok so far, but you should override the core
JMail with your modfied version as I described earlier.

Regards,

[elin]

The other possibility of course is to commit to working on JMail so it is modernized and more usable. Doing that would mean you don't have to worry about core updates because you wouldn't want or need to hack core files and you would be helping everyone and getting help from everyone which is good for you long term too.

Elin

[garyamort]

On Sunday, September 4, 2011 11:05:26 PM UTC-4, elin wrote:

The other possibility of course is to commit to working on JMail so it is modernized and more usable.

I'm not sure others would really like my direction on Messaging and EMail.  I'll try to wrap it all up and setup a branch and fork on github and it can be reviewed.

I'd be happy to have them go directly to core. My vision is very different than the current implementation - and while it leads to a more robust system, if it was done in core it would break anything which relies on the fact that JMail is derived from PHPMailer.

I love PHPMailer, but I don't like being dependent on it - it has way too many features that are simply not necessary for most websites - and it lacks some basic features that are.