Events should pass it's arguments by reference

[ProjectOrangeBox]

---

name: Bug report
about: Events should pass it's arguments by reference

---

Direction

Describe the bug
When a event is triggered it should pass it's arguments by reference.
This will allow other listeners down the priority "chain" to modify the arguments as needed.
The way it is written right now trigger only returns a boolean therefore it's impossible to grab a value that might have been changed via a trigger.

For example if I setup the following

/* some where in the code */

event::on('example',function($arg1,$arg2) {
	$arg1 = $arg2.$arg1.$arg2;
},EVENT_PRIORITY_HIGH);

/* somewhere else in the code later */

event::on('example',function($arg1,$arg2) {
	if (substr($arg1,0,strlen($arg2)) != $arg2) {
		$arg1 = '<b>'.$arg1.'</b>';
	}
},EVENT_PRIORITY_NORMAL);

Then call

$a = 'Hello World!';
$b = '<i>';


$boolean_success = event::trigger('example',$a,$b);

Then the EVENT_PRIORITY_HIGH event will trigger first and in affect down the listener priority chain when it calls the normal priority listener nothing happens.

But let's say the higher priority event wasn't loaded for a certain page then the normal priority event would be called.

Without passing these arguments by reference there would be no way to modify the arguments to have them further modify down the chain if needed. This will allow CodeIgniter Events to handle more advanced triggering. Where now it's more of a all or nothing kind of trigger. The trigger is fired and you get input but you can't actually do anything with the input because it's passed by value.

I believe you would only need to change the trigger declaration by adding the &

public static function trigger($eventName,&...$arguments): bool

CodeIgniter 4 version
4.0.x

Affected module(s)
N/A

Expected behavior, and steps to reproduce if appropriate
See above Describe the bug

Context
N/A

[jim-parry]

@lonnieezell I don't see any side effect to this, and am ok with it.

[lonnieezell]

Does passing a reference to a packed array actually work?

If it does, I'm fine with a PR for this. We looked at this at one point and came to the conclusion at that point that we didn't want to change the way the arguments were passed (via argument packing/unpacking) so it wouldn't work. If this works, though - definitely.

@ProjectOrangeBox have you tried this?

[ProjectOrangeBox]

Lonnie, passing in a object certainly works: <?php namespace App\Controllers; use CodeIgniter\Controller; use CodeIgniter\Events\Events; class Home extends Controller { public function index() { Events::on('register',function($params) { echo 'inside trigger: '.$params->value.'<br>'; $params->value = 'foobar'; }); $test = new foobar; echo 'before trigger: '.$test->value.'<br>'; Events::trigger('register',$test); echo 'after trigger: '.$test->value.'<br>'; } } class foobar { public $value = '123'; } But passing in a array not so much <?php namespace App\Controllers; use CodeIgniter\Controller; use CodeIgniter\Events\Events; class Home extends Controller { public function index() { Events::on('register',function($params) { echo 'inside trigger: '.$params[0].'<br>'; $params[0] = 'foobar'; }); $test[] = 'foobar'; echo 'before trigger: '.$test[0].'<br>'; Events::trigger('register',$test); echo 'after trigger: '.$test[0].'<br>'; } } (Those are simply modifying the CI4 Home Controller) Don Myers

…

On Oct 23, 2018, at 11:31 PM, Lonnie Ezell ***@***.***> wrote: Does passing a reference to a packed array actually work? If it does, I'm fine with a PR for this. We looked at this at one point and came to the conclusion at that point that we didn't want to change the way the arguments were passed (via argument packing/unpacking) so it wouldn't work. If this works, though - definitely. @ProjectOrangeBox <https://github.com/ProjectOrangeBox> have you tried this? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1298 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ANN53W0-AEHVWe2lw76lSXIWSvYNCovWks5un98pgaJpZM4XKoxF>.

[lonnieezell]

You didn't check the argument packing/unpacking version :)

I ran a quick test and it does appear to work. So looks like an easy change.

[ProjectOrangeBox]

Lonnie, I’m sorry I completely miss understood the verbage. I had no idea the … syntax was technically called packing & unpacking. Thank you for the heads up. So does this mean adding “pass by reference” will be added. Don Myers

…

On Oct 24, 2018, at 4:02 PM, Lonnie Ezell ***@***.***> wrote: You didn't check the argument packing/unpacking version :) I ran a quick test ***@***.***/ArgumentPackingReferenceCheck?language=php> and it does appear to work. So looks like an easy change. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1298 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ANN53ZL9jB7-O3NwwdwVRQd11-uzypCkks5uoMdegaJpZM4XKoxF>.

[lonnieezell]

@ProjectOrangeBox No worries. I've got to run a couple of more tests with it, but looks like it should be good. I honestly didn't expect that to work, but looks like it does.

[jim-parry]

Dropped. See #1289