Closed dmolineus closed 9 years ago
First off, nice idea of providing a central API. The whole purpose of these events is to abstract the Contao API from other projects to have a single point of abstraction. when a parameter change within Contao occurs, the event bindings take care of handling the incompatibility. By providing a function based app you are recreating the problem we are trying to solve. Now it is again impossible to drop parameters etc.
On the other hand, I (and others as well) dislike functions in name spaces and prefer to have them in a wrapper class. This is also the reason why I never implemented style checks for it.
Okay, so I missed the real purpose behind it. In fact, at least this is my opionion, the problem you are trying to solve is just moved to the event constructors. They have the arguments as well. The api I proposed is just a mirror of the provided constructors.
So dropping parameters is already an issue. Adding new is possible as well. Of course having the message objects provides more flexibility.
I know that namespaced functions are not preffered most of times. Well, it's just an implementation detail. Could change it easily.
To gain more flexibility with arguments changes (keep it limited only by the event classes) the event could be passed as argument.
ContaoCommunityAlliance\Contao\Bindings\Api\Backend::addToUrl(new AddToUrlEvent($event));
@tristanlins, what do you think about his approach?
the problem you are trying to solve is just moved to the event constructors
Personally, this is one point I dislike in our implementation. I will answer your approach with a cite:
Most small code fragments are not worth being abstracted.
You still reintroduce the old problem: A static, unflexible, bricky API.
At all, using events to overcome the API differences in the different Contao Versions was a bad idea. As @discordier said: "The whole purpose of these events is to abstract the Contao API from other projects to have a single point of abstraction."
I'm hopeful Contao 4 provide it's own events and drop the static API at all.
Thx for your work, but I dislike it at all. No matter if the functions are "implementation detail", classes with static functions are not better, expect there is a class around the functions ;-D
The current implementation of the event bindings provides a flexible way to manipulate the result of the actual called Contao API. Unfortunately the API get lost, so every call has to dispatch the event for itselves.
Before using another library for that I provide it as a PR. This PR propose an api which hides the event dispatching. So every call could be simplified:
There are also unit tests provided. They test the event dispatching and the correct returning of the value.
The current implementation uses the functional approach. I think it better fits the need. If you general like the idea but not the functional solution we could discuss it.
The current build task will fail because the used code sniffer (v1.x) cannot handle namespaced functions. All other rules are fullfilled.