Questions on HookEvent::arguments()

[suntrop]

Hi all

I am working on my knowledge of PW's hooks … and maybe on PHP OOP in general  Now I have got two questions on this hook:

// From basic-page.php template
$page->price(3, true);

// From my module's init()
$this->addHook('Page::price', $this, 'price');

// and the method
function price($event) {

    $page     = $event->object;
    $quantity = $event->arguments(0);
    $vat      = $event->arguments(1) ?? false;

    // Get tier price
    $unitprice = $this->tierprice($event);

    $event->arguments(1) = $unitprice; // possible/correct?
    if ($vat) $unitprice = $this->addVAT($event);

    return $unitprice;
}

 

FIRST

Why does the PageArray comes from $event->object and where do know this from (beside just copying code from other forum posts ?) And why is $event->arguments(0) the first variable I pass to my method and not the $page as it says on that doc page: https://processwire.com/api/hooks/#read_arguments

SECOND

I would like to pass the complete $event (the $page and my arguments) to the addVAT() method. But in that method I need the $unitprice as well. Is it possible and the correct way to just create a new third argument ($event->arguments(2) = $unitprice)? Or do I need to write something like $unitprice = $this->addVAT($event, $unitprice)?

 

[BitPoet]

30 minutes ago, suntrop said:

FIRST

Why does the PageArray comes from $event->object and where do know this from (beside just copying code from other forum posts ?) And why is $event->arguments(0) the first variable I pass to my method and not the $page as it says on that doc page: https://processwire.com/api/hooks/#read_arguments

SECOND

I would like to pass the complete $event (the $page and my arguments) to the addVAT() method. But in that method I need the $unitprice as well. Is it possible and the correct way to just create a new third argument ($event->arguments(2) = $unitprice)? Or do I need to write something like $unitprice = $this->addVAT($event, $unitprice)?

First:

You know what $event->object is because it is an object of the very class you're hooking onto. In the linked docs, the hook is on Pages::saved, so the event object is an instance of Pages (not a PageArray). In your code, you're hooking Page::price, so $event->object is a Page object. If your hook was InputfieldText::firstWord, it would be an InputfieldText instance.

Second:

I'd avoid passing $event around to other methods. It kills readability so $event should only be used in the hooks themselves, not in further methods or functions. Yes, you can manipulate arguments. The syntax is in the docs section you linked, in your case it would be $event->arguments(2, $unitprice). But that is mostly meant for before hooks where you want to change the behavior of the original, hooked method.

It would be better though to pass all necessary arguments to tierprice and addVat one by one instead of manipulating the "magic thing" that is $event, i.e. something like:

$unitprice = $this->tierprice($page, $quantity);

Best practice to keep code readable is to use a pattern with a public hook method that decorates your hooked class and a couple of private methods which do the work.

<?php

/* The usual module stuff... */
public function init() {
  $this->addHook("Page::price", $this, "hookPrice");
}

public function hookPrice(HookEvent $event) {
  $page = $event->object;
  $quantity = $event->arguments(0);
  $vat = $event->arguments(1) ?? false;
  
  $unitprice = $this->tierprice($page, $quantity);
  if($vat) $unitprice = $this->addVat($unitprice);
  
  $event->return = true;
}

private function tierprice($page, $quantity) {
  /* do whatever calculations and return the result */
}

/* ... */

 

[suntrop]

Thanks a lot! The first point took me some time, but it is really obvious now  

The second one though isn't that clear to me. I have code more or less like your example but thought passing the $event always is better, because I can use those methods on its own. While it isn't necessary in the code section above, I have quite a couple of methods I need/want to use directly. 

I could create a hook for tierprice() as well, and call it $page->tierprice($page, 3) — but this seems wrong to me too. You would create a new method like getTierprice() which in return calls the private method tierprice()?

[LostKobrakai]

16 hours ago, suntrop said:

And why is $event->arguments(0) the first variable I pass to my method and not the $page as it says on that doc page: https://processwire.com/api/hooks/#read_arguments

It just happens to be that the first argument to the function hooked in that part of the documentation is a page object. So it's always about the functions arguments.

1 hour ago, suntrop said:

The second one though isn't that clear to me. I have code more or less like your example but thought passing the $event always is better, because I can use those methods on its own. While it isn't necessary in the code section above, I have quite a couple of methods I need/want to use directly. 

There are a few things wrong with passing around the hook event object:

The hook event object is used only in combination with a hook. So every code touching that object automatically binds itself to a usage only through a hook, which is a unneccesary coupling. This violates the open/closed principle (O if SOLID), because if anything related to hooks does change you need to modify each method receiving the hook event and not only the one method, which is triggered by the hook. On the other hand if any of your business logic does change you should not need to modify code, which is only there to make sure your hook is handled.

When you're not complying to the above it's also way easier to get lost in where things do actually change in regards to handling a hook. You wouldn't want to be in a situation, where you know a hook does cause issues, but you cannot find the place where that hook is manipulated incorrectly, because the objects is passed around in so many places. Metaprogramming (hooks are kind of that) is manipulating code at runtime, which is a responsibility to keep up to. This is best done by keeping the surface of code actually manipulating other code as small as possible.

On the other hand if your hook callback function is only dealing with "making the hook work" and passing all other responsibility of to other functions – just passing data gathered from the $event object, but not the event itself – you automatically have functions, which you can easily use from places other than that hook as well. Say you later notice that you need to use that tierprice() method from another hook with different arguments or in some other part of your business logic, which is not triggered by a hook at all. You wouldn't want to duplicate the function, just to be able to use it from different places. You also don't want to fabricate a fake $event object just to call it. 

[SOLID Principles] https://en.wikipedia.org/wiki/SOLID_(object-oriented_design)

[suntrop]

41 minutes ago, LostKobrakai said:

On the other hand if your hook callback function is only dealing with "making the hook work"

So, if I create hooks with the pure purpose of passing $event to other private methods, "it is ok"?

On my current example above the tricky part is, almost everything builds up from the tierprice, wich in return builds up the unit price, wich builds the price, … Sometimes I need the total, sometimes I need to get the unit price and sometimes I need the with or without VAT. Like a pyramid.

On my very first draft I had a couple functions in _function.php, then I moved on to create all sorts of different/independent hook methods but a lot of methods did the same thing, just with some slightly other output. At the end I thought, what I nice idea to just pass always the $event around  

[LostKobrakai]

On 5/5/2018 at 11:52 AM, suntrop said:

So, if I create hooks with the pure purpose of passing $event to other private methods, "it is ok"?

The purpose of the hooks callback is not to pass the $event object forward, but rather to handle everything related to it. It should unpack all the data you need from the $event, call any external functions it needs to call by using that retrieved data (not the $event itself) and use the returned values to put them back into $event if needed. With the exception of this hook callback it's very unlikely any code would need to know about $event and if it's triggered by a hook or not.