Changes to NOTIFY_ORDER_AFTER_SEND_ORDER_EMAIL

[swguy]

In 1.5.4 and below, this notifier is called with an array. In 1.5.4, see includes/classes/order.php line 1099.
In 1.5.5a, the array was removed and the parameters were just added individually.

Why was this change made?
How could this have been better documented?

[lat9]

@swguy, it's not a matter of documentation since changing the parameters to a notify action is like changing the parameters to a function (with the associated fall-out for any plugins).

[swguy]

I understand how to change the parameters to a function. I'm just suggesting that the change be announced. In this case, I had to discover it by tedious debugging, since this unannounced change broke my logic.

In fact, a change to a function call would have been preferable because that would immediately lead to a debug log for parameter incompatibility. In this case, no such log was issued.

[DrByte]

In 1.5.4 and below, this notifier is called with an array. In 1.5.4, see includes/classes/order.php line 1099.
In 1.5.5a, the array was removed and the parameters were just added individually.

Why was this change made?

Passing data as an array leaves it immutable, but passing data as params 2-9 allow the var to be altered and the changes passed back to the calling code (if received into the observer by reference).
When passing notifier parameters nowadays I usually put the immutable item (such as order-id) that shouldn't be allowed to change as the first data param, and then additional data points as subsequent params so that they can be received-by-reference in case it makes sense for them to be changed.

In the case of NOTIFY_ORDER_AFTER_SEND_ORDER_EMAIL it's almost negligible either way, but I imagine it got changed for the sake of consistancy while others were being added/updated in that same class.

[swguy]

With respect, I think this was an error that should be avoided in future updates.

I am baffled by your first assertion. Function arguments are passed by value in PHP. You can't change the value of a parameter unless you precede it by &. They are immutable by default.

Making a *breaking change* for the purpose of consistency is really not a great tradeoff.
What's the value of consistency vs the value of having things work predictably?

And here's the thing: this is not even a consistent change! Lines 886 and 1085 also pass arguments as an array.

Here's my point: the notifiers were introduced as a way of making it possible for people to make modifications without changing core files. But if you can't count on the notifiers working consistently from release to release, why would you use them? It's not like I'm using a private API or something - this is the public interface to Zen Cart.

If you were to create new notifiers using a standard pattern (pass args by array or whatever) and deprecate the old ones over a series of releases with supporting documentation indicating how to upgrade, and then change the major number of the release (since it's a breaking change), well, that would be consistent with how commercial SDKs are updated over time. I'm just saying that it's not reasonable to willy-nilly change the way a program interface works based on your evolving design preferences.

[DrByte]

Point well made.

Apologies for the inconvenience that the change has evidently caused you.