Jaxon and the factory pattern

[iReserveApps]

Hi, Seering several topics on the register classes, which has been changed in V3 and enforced in V4. https://github.com/jaxon-php/jaxon-core/issues/116 https://github.com/jaxon-php/jaxon-core/issues/113 The explanation is clear, it is a design choice to change Jaxon.

We are using quite a large framework (with xajax from the early days, approx 4k ajax calls present) and using a Factory pattern https://en.wikipedia.org/wiki/Factory_(object-oriented_programming) This pattern causes the objects to be populated. Even custom objects can be created, depending on environment.

The register of classe just by name is causing the factory to be skipped. The instance (at time of register) is already nicely populated. Jaxon is now enforcing to work around this. For us, its already clear this is a blocker, we cannot use jaxon the way it is designed.

Probably you have thought about it, but it might be nice to hear about this issue. That's why we post this issue.

[feuzeu]

Hi, According to what you just said, I see no reason why you couldn't use the latest Jaxon release. On the contrary, having a factory may make things easier.

First of all, you don't need to instantiate your classes when registering them, be it with a factory or by any other mean. Providing class names, or even better, a directory and a namespace, is all what is required. https://www.jaxon-php.org/docs/v4x/registrations/classes.html https://www.jaxon-php.org/docs/v4x/registrations/directories.html https://www.jaxon-php.org/docs/v4x/registrations/namespaces.html

Now when processing a request, you simply need to provide the Jaxon DI component with a mean to get an instance of any of your classes from your factory.

jaxon()->di()->set($jaxonClassFullName, function($di) {
    // Return an instance of the class using the factory.
});

See https://www.jaxon-php.org/docs/v4x/features/dependency-injection.html for the documentation. The trickiest part here is that you will need to do that for all the classes you want to export to javascript. As an alternative, you can also replace your factory with the Jaxon DI component. I don't know how hard this may be.

[iReserveApps]

Thanks for mentioning this alternative. Dependency injection is something we did not explore yet. As you say, it has to be done for all classes. We know about the documentation on registration, we only use callable functions. By making use of factory, all init stuff is done in the framework in the bootstrap phase. Jaxon is called after that bootstrapping. It will require a framework redefinition to cope with an alternative approach.

You say: "you dont need to instantiate your classes". Well we do need to do that. Classes are not empty, they contain logic during initialising them. Including determining the correct class names, context, environment. All this is done in the factory. And that has to be done exactly one time. From that moment use the factory.

We tried in a local fork to add the feature (back) to the Jaxon\Plugin\Request\CallableFunction Plain and simple

if ($sName == 'instance') $xCallable->configureClass($sName, $sValue); else $xCallable->configure($sName, $sValue);

and then in the call $sClassName = $this->xPhpFunction[0]; if ($this->instance instanceof $sClassName) $this->xPhpFunction[0] = $this->instance; else $this->xPhpFunction[0] = new $sClassName;

This way we can cope with the complete framework in one go, we can now replace all the register calls in one replacement.

Honestly, I don't understand why a framework should enforce this. Jaxon is a framework and will be embedded in other frameworks. Like I said before, you probably have a reason for changing this feature from v2 to v3, but it's unclear if there is an objective reason for it.

[feuzeu]

If you are only using callable functions, that means you have registered each method you want to call from the browser as a callable function, right?

Even in this case, you don't need to instantiate your classes when registering the functions. I know Xajax allowed that, but it is simply not necessary. Pass the class name to the register() method and you are done. So you should be able to register your functions and display your pages without any issue.

The rationale behind that is Jaxon will instantiate only the class that is called when processing the ajax request. Instantiating the other classes in this case is useless, and just makes your application slower.

Now of course you are right about the fact that the callable function plugin currently tries to new the class when processing the request, but that is a bug. It needs to check first if the class is available in the DI container. It is very easy to fix.

[iReserveApps]

If you are only using callable functions, that means you have registered each method you want to call from the browser as a callable function, right?

That is right.

I know Xajax allowed that, but it is simply not necessary

To be honest, thats an opinion. In our framework it is needed.

So you should be able to register your functions and display your pages without any issue.

We understand your documentation and way of registering, but the whole point of this issue is that it IS an issue if you do a blank init of a class from without the framework

Instantiating the other classes in this case is useless, and just makes your application slower.

That is something we have covered in the framework and this is not happening. So not an issue.

I wanted to help you as I believe you have done a good thing by maintaining Jaxon. So this issue is providing maybe a new perspective. Based on your answers, I do understand your rationale. I do understand how you try to enforce a way of working you feel is the best practice. But maybe there are multiple ways in achieving a best practice. I hope you can see that. Enforcing one way is blocking another way. (An D.injection is adding too much of complication right now).

So we continue on the fork for now. Thanks for your efforts and replies!

[feuzeu]

You can't just decide that something is an issue without explaining why. So let me ask you a question. When you register your functions, what piece of information do you need that you can't have without instantiating your class? Seems to me that your function names is all you need, and you already know them.

If your framework absolutely needs you to do something that is absolutely not necessary, it sounds to me like a design flaw you need to address. You cannot just ask the library to adapt to that.

DI is too complicated? Sorry, but there's a reason why every single PHP framework out there now use it.

At the end of the day, you just need to make your app work. Since it already does, it's ok to me.

[iReserveApps]

Dear Feuzu, I did try to explain in a very respectfull way. Apparently that was not clear to you. The fact that you see it as a possible design flaw is less respectfull.

You mention a lack of explanation. At the same time, you seem not to be able to explain why you have enforced your way of working. You just have an opinion and say "it is not needed".

DI is not technically complicated but does require a serious efforrt in the 4k classes we maintain. So it seems pretty fair to take into consideration the effort it requires. Especially if there is no real reason. Definitely not a design flaw.

Well, we tried to contribute here. That at least failed. Wish you good luck.

[feuzeu]

First you can't make the effort to spell my name correctly, then you say that not agreeing with you is disrespectful. Ok. Ok.

[iReserveApps]

I did not say disagreeing is disrespectful, saying it is a design flaw is.

Apologies for misspelling your name. Its alias says "feuzeu".

Off course we dont know each other, but if you read again we are trying to be respectful.

Finally: you did shut down an option which was in the framework before. If someone then responds to that ( I am not the first), you might expect that. It worked, and then it does not work anymore. I do understand that can be a choice. The PHP language changes also, also with breaking changes. You are absolutely in a position to make the choice to do such a change. You have the right.

Maybe feedback might add something. And if you are not open for it, fine. Don't get offended if someone gives feedback.

[feuzeu]

If saying a piece of software has an issue is disrestpectful, why would you be allowed to post an issue here? Why would anyone be allowed to post an issue on any software?

If you really think that, please stop reading here, because I'm going to be more "disrespectful".

You keep complaining about your 4k classes. But why the hell are you still registering them individually today?

Since version 1, which was published more than 7 years ago now, you are adviced when you have that many classes to move them to a directory which you register with Jaxon. Since then, that feature has been improved to a point where it works quite well today.

It is not only my choice, it is also the way modern PHP works. Because when doing so, PHP can take advantage of autoloading and namespacing to find and load your classes more easily and more efficiently. That's the reason why PSR-4 is the recommended way to write PHP classes nowadays.

You had many years to upgrade your classes, since you initially wrote them for Xajax. I your classes are not PSR-4 compliant, which I guess from what you said, you should consider an upgrade. After that, you will be able to register them as one directory with one namespace. In the worst case, you may just have a few namespaces.

If you don't want to, it is up to you. But just know that I consider that as a bad design choice, and no, no, no, I will not downgrade this library to cope with that.

The library source code is available. It is your right to adapt it to your needs if your prefer, as you said you did. It is ok for me. Since it has worked, for me there is no more issue.

[edrobinson]

Thierry,

While I agree with you in principle I cannot agree with the idea that we should all do it this way because that is how it is done these days. My mother used to say "If all of the other kids wanted to jump off of a cliff, would you want to too?"

If no one tried something different, we would still be cooking over a campfire in the dark.

As you know, I use Jaxon in a single class per page mode involving what would be called a "hack" and now I am also registering the same single function in every one of my classes. I didn't need a hack in Jaxon 2X but you chose to make a breaking change starting in V3.

This results in faster, cleaner code so that it doesn't matter if there are only 3 methods to be called or 4K, as in the present issue, there is no overhead. You also know where all of a page's code lies and can make changes or additions with ease.

While I do use composer autoloading with classmap, I still cannot get my head around namespaces so PSR-4 is still out. This is probably due to my advanced age. I will turn 80 in the coming March. Maybe that isn't a good excuse but it's the only one I can come up with. :)

Ed

On Sat, Oct 21, 2023 at 12:04 PM Thierry Feuzeu @.***> wrote:

If saying a piece of software has an issue is disrestpectful, why would you be allowed to post an issue here? Why would anyone be allowed to post an issue on any software?

If you really think that, please stop reading here, because I'm going to be more "disrespectful".

You keep complaining about your 4k classes. But why the hell are you still registering them individually today?

Since version 1, which was published more than 7 years ago now, you are adviced when you have that many classes to move them to a directory which you register with Jaxon. Since then, that feature has been improved to a point where it works quite well today.

It is not only my choice, it is also the way modern PHP works. Because when doing so, PHP can take advantage of autoloading and namespacing to find and load your classes more easily and more efficiently. That's the reason why PSR-4 is the recommended way to write PHP classes nowadays.

You had many years to upgrade your classes, since you initially wrote them for Xajax. I your classes are not PSR-4 compliant, which I guess from what you said, you should consider an upgrade. After that, you will be able to register them as one directory with one namespace. In the worst case, you may just have a few namespaces.

If you don't want to, it is up to you. But just know that I consider that as a bad design choice, and no, no, no, I will not downgrade this library to cope with that.

The library source code is available. It is your right to adapt it to your needs if your prefer, as you said you did. It is ok for me. Since it has worked, for me there is no more issue.

— Reply to this email directly, view it on GitHub https://github.com/jaxon-php/jaxon-core/issues/121#issuecomment-1773880491, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAFCS576QGYACI6KXANAQVLYAQFEPAVCNFSM6AAAAAA6HWKR4OVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTONZTHA4DANBZGE . You are receiving this because you are subscribed to this thread.Message ID: @.***>

-- Ed Robinson Ph: 208-376-2680

[feuzeu]

Ed,

I said Because when doing so, PHP can take advantage of autoloading and namespacing to find and load your classes more easily and more efficiently. It is not a matter of preference.

Why did your mother ask you that question? Because you worked hard? Because you took care of your family?

You cannot ask to change a library people are using, some for their businesses or their work, just to make your "hack" to work. It's crazy.

There are well-known best practices that most people, including me, try to respect. You are free to compare that with jumping off a cliff, but please allow me to disagree with you.

[edrobinson]

Thierry,

Thanks for replying. I do use autoloading and it is very useful in my opinion. Like I said, I have yet to fully understand namespacing so I do not use PSR-4 autoloading.

My mother would ask me this question when I wanted to do something just because everyone else was doing it.

I am not asking you to change the library. I think that would be a high level of arrogance. I am, however, looking for a workaround that makes it easier to use the library the way I would like to. I do keep running into problems with my hack and may ultimately have to do it the way you recommend or just go back to V2.X where I am most comfortable.

I do not compare best practices with jumping off a cliff. I get the feeling from you that you do not approve of trying anything outside of best practices. That's how good changes come about, although, with some rough roads along the way.

I will keep studying your code to better understand it and perhaps I will find a way.

Regards, Ed

On Sun, Oct 22, 2023 at 4:25 PM Thierry Feuzeu @.***> wrote:

Ed,

I said Because when doing so, PHP can take advantage of autoloading and namespacing to find and load your classes more easily and more efficiently. It is not a matter of preference.

Why did your mother ask you that question? Because you worked hard? Because you took care of your family?

You cannot ask to change a library people are using, some for their businesses or their work, just to make your "hack" to work. It's crazy.

There are well-known best practices that most people, including me, try to respect. You are free to compare that with jumping off a cliff, but please allow me to disagree with you.

— Reply to this email directly, view it on GitHub https://github.com/jaxon-php/jaxon-core/issues/121#issuecomment-1774216052, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAFCS54T7O5TIZGGZ47UQOLYAWMMDAVCNFSM6AAAAAA6HWKR4OVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTONZUGIYTMMBVGI . You are receiving this because you commented.Message ID: @.***>

-- Ed Robinson Ph: 208-376-2680

[iReserveApps]

Hi,

The topic has been going into a PSR4 discussion. It seems the conclusion is that it is about loading a class. Yes, the issue is that Jaxon is using a "new Class" statement so I understand that seems to be connected to autloading.

But the original (our) problem is:

• in our controller we first load a few classes via an autoloader which checks for certain environment related stuff
• depending on the environment the list of classes can differ
• we instantiate the classes via class Auttoloader, like "Autoloader::Factory(classname)" or "Autoloader::singleton(classname)"
• In most of the constructor calls there are parameters, like context and id which are set upon loading, all centrally maintained and given to the constructor calls

When applying v4 of jaxon (views only, not part of models), the issue lies within the lack of parameters in the constructor calls. So we want to initiate the classes (not all but the ones needed off course) earlier than the loading of the jaxon framework.

It is a bit unclear why this is going into the PSR4 discussion. The issue is not really the best practice on namespace loading, it is about handling parameters and keep track of loaded classes. A small example which does use parameters: https://www.phptutorial.net/php-oop/php-composer-autoload/ Which is not the way we work btw. But it does show a way to pass params.

It is true that our autoloader is not fully PSR4 ready, but even when it is (and that will happen) we do not want to remove the option for parameters. It is not a bad practice to do so. Also not part of the PSR4 requirements.

I have also taken a look at DI. Nice that you solved that bug the last few days! DI is another approach. Yes, I do see the purpose of that approach as well. Just as well as our change to the callable function is working (by using instances). Our change is not bad and should not be downsized to " then you are not complying with PSR 4 so its a design flaw ". That is simplifying the issue and putting the discussion in a corner "who is doing the best within best practices".

Thanks so far!

[edrobinson]

iReserveApps:

Your statement: "Yes, the issue is that Jaxon is using a "new Class" statement so I understand that seems to be connected to autoloading." is right on except that it is not autoloading but instancing a new copy our classes.

I instance Jaxon in the "view" method of my classes. I then register my "dispatch" function as the only public method. Then I use Smarty to generate the page. Page setup calls the Jaxon CSS and JS generation methods and assigns them to Smarty variables to transport the code to the client. That works fine. The call to my method appears in the HTML.

The problem arises when Jaxon instances my class using new. There is no "memory" of my method registration and I keep bumping into the problem no matter what I try. Several times now I thought I had figured out a civilized hack only to get an infinite loop error if I register in my class constructor or no processor defined for my registered method if I only registered in the page generation code.

I don't know why this change was made going from V2 to V4 but it really messed up my life! :(

My code base is small so I don't face the issue around 4K calls. I have created a version using the 3 file/page approach but it does not sit well. Fortunately, using Smarty takes care of the separation of concerns.

Right now I see 3 choices: 1. Return to Jaxon V2.X or 2. Use the separate files approach or 3. Keep on looking for the holy grail hack.

Ed.

[iReserveApps]

@edrobinson

Looking for the holy grail hack, here it is `Index: vendor/jaxon-php/jaxon-core/src/Plugin/Request/CallableFunction/CallableFunction.php

--- vendor/jaxon-php/jaxon-core/src/Plugin/Request/CallableFunction/CallableFunction.php (revision 18332) +++ vendor/jaxon-php/jaxon-core/src/Plugin/Request/CallableFunction/CallableFunction.php (revision 18333) @@ -57,6 +57,7 @@

• @var array */ private $aOptions = [];

  • private $instance;

    /**

• The constructor @@ -127,6 +128,17 @@ }

  /**

  • • @param string $sName
  • • @param object $sValue
  • • @return void
  • */
  • public function configureClass(string $sName, object $sValue)
  • {
  • $this->xPhpFunction = [$sValue::class, $this->xPhpFunction];
  • $this->instance = $sValue;
  • }
  • /**
• Call the registered user function, including an external file if needed
• and passing along the specified arguments

• @@ -144,7 +156,8 @@ if(is_array($this->xPhpFunction) && is_string($this->xPhpFunction[0])) { $sClassName = $this->xPhpFunction[0];

  • $this->xPhpFunction[0] = new $sClassName;
  • if ($this->instance instanceof $sClassName) $this->xPhpFunction[0] = $this->instance;

  • else $this->xPhpFunction[0] = new $sClassName; } return call_user_func_array($this->xPhpFunction, $aArgs); } Index: vendor/jaxon-php/jaxon-core/src/Plugin/Request/CallableFunction/CallableFunctionPlugin.php

    --- vendor/jaxon-php/jaxon-core/src/Plugin/Request/CallableFunction/CallableFunctionPlugin.php (revision 18332) +++ vendor/jaxon-php/jaxon-core/src/Plugin/Request/CallableFunction/CallableFunctionPlugin.php (revision 18333) @@ -178,7 +178,9 @@ $xCallable = new CallableFunction($sFunction, $this->sPrefix . $sFunction, $this->aFunctions[$sFunction]); foreach($this->aOptions[$sFunction] as $sName => $sValue) {

  • $xCallable->configure($sName, $sValue);
  • if ($sName == 'instance') $xCallable->configureClass($sName, $sValue);
  • else $xCallable->configure($sName, $sValue);

  • } return $xCallable; } `

Only the two files Callable function have been changed, this works like a charm.

[iReserveApps]

After applying that change, call it with "instance" as a parameter

$jaxon->register(\Jaxon\Jaxon::CALLABLE_FUNCTION, "showBatchDetail", ["instance" => $this]);

[edrobinson]

Thank you for this information. I assume it is from a version control file.

While I hate to admit it, I do not use any formal version control as It's just me in this shop so I make copies as the need strikes me.

Would you be so kind as to interpret the changes I need to make?

Regards, Ed

On Mon, Oct 23, 2023 at 11:09 PM iReserveApps @.***> wrote:

After applying that change, call it with "instance" as a parameter

$jaxon->register(\Jaxon\Jaxon::CALLABLE_FUNCTION, "showBatchDetail", ["instance" => $this]);

— Reply to this email directly, view it on GitHub https://github.com/jaxon-php/jaxon-core/issues/121#issuecomment-1776547447, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAFCS5Y432RWE34E325JARTYA5ERZAVCNFSM6AAAAAA6HWKR4OVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTONZWGU2DONBUG4 . You are receiving this because you were mentioned.Message ID: @.***>

-- Ed Robinson Ph: 208-376-2680

[iReserveApps]

Hi Ed In the CallableFunctionPlugin.php change

$xCallable->configure($sName, $sValue); into if ($sName == 'instance') $xCallable->configureClass($sName, $sValue); else $xCallable->configure($sName, $sValue);

In the CallableFunction.php add property private $instance;

add function `/**

• @param string $sName
• @param object $sValue
• @return void */ public function configureClass(string $sName, object $sValue) { $this->xPhpFunction = [$sValue::class, $this->xPhpFunction]; $this->instance = $sValue; }`

and change function call into public function call(array $aArgs = []) { if(($this->sInclude)) { require_once $this->sInclude; } // If the function is an alias for a class method, then instantiate the class if(is_array($this->xPhpFunction) && is_string($this->xPhpFunction[0])) { $sClassName = $this->xPhpFunction[0]; if ($this->instance instanceof $sClassName) $this->xPhpFunction[0] = $this->instance; else $this->xPhpFunction[0] = new $sClassName; } return call_user_func_array($this->xPhpFunction, $aArgs); }

[feuzeu]

FYI, there's solution to this issue that works well with the latest library version. Register the app function with the class name, and register the class in the DI.

jaxon()->register(Jaxon::CALLABLE_FUNCTION, $sFunctionName, ['class' => $sClassName]);

jaxon()->di()->set($sClassName, function($di) {
    // Return an instance of the class.
});

It is even recommended to instead register the app services in the DI, and pass them to the Jaxon class contructors. No need to register the Jaxon classes themselves.

There's no serious reason why someone would want to instantiate 4k classes (plus the 4k classes needed to make it work) unnecessarily!!! So don't do like @iReserveApps!!!

[iReserveApps]

I never said we instantiate 4k classes @feuzeu ! Stop jumping to conclusions, we have just tried to help.

The DI solution did not work when the original post was made. Nice that it does now. Thanks for the fix on that.

All together this was the solution we are looking for.

[edrobinson]

Thanks. I have implemented the changes and it works well.

Ed

On Wed, Oct 25, 2023 at 2:24 AM iReserveApps @.***> wrote:

Hi Ed In the CallableFunctionPlugin.php change

$xCallable->configure($sName, $sValue); into if ($sName == 'instance') $xCallable->configureClass($sName, $sValue); else $xCallable->configure($sName, $sValue);

In the CallableFunction.php add property private $instance;

add function /* @param string $sName @param object $sValue @return void */ public function configureClass(string $sName, object $sValue) { $this->xPhpFunction = [$sValue::class, $this->xPhpFunction]; $this->instance = $sValue; }

and change function call into public function call(array $aArgs = []) { if(($this->sInclude)) { require_once $this->sInclude; } // If the function is an alias for a class method, then instantiate the class if(is_array($this->xPhpFunction) && is_string($this->xPhpFunction[0])) { $sClassName = $this->xPhpFunction[0]; if ($this->instance instanceof $sClassName) $this->xPhpFunction[0] = $this->instance; else $this->xPhpFunction[0] = new $sClassName; } return call_user_func_array($this->xPhpFunction, $aArgs); }

— Reply to this email directly, view it on GitHub https://github.com/jaxon-php/jaxon-core/issues/121#issuecomment-1778763058, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAFCS57FO4AQMDMMIVUMYFTYBDEEHAVCNFSM6AAAAAA6HWKR4OVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTONZYG43DGMBVHA . You are receiving this because you were mentioned.Message ID: @.***>

-- Ed Robinson Ph: 208-376-2680