Callbacks Classes

[yak0d3]

While working on the "callbacks loaders" feature, i had the idea of implementing a loadCallbacksFrom method. This method parses all "callbacks classes" in a given path.

I thought of implementing a "Callback contract" that forces a handle method that takes 0 arguments. However, this way we'll need to deffer between the callables and the "callabacks classes" when saving the callback and its arguments here and also when loading the callbacks proxies here.

This will surely cause some performance drop as we do have to make some more checks, and in contrast it would be nice to have callbacks classes.

Do you think it is worth it? Or have another idea on how to make this work better?

[axlon]

I think adding an interface for this goes against the "Laravel" way of doing things; it takes away the ability to dependency inject into the handle method.

In my personal opinion we shouldn't allow adding bulk callbacks, because:

1. Most applications won't need that many callbacks anyway
2. Its not that much effort to register them one by one
3. I think its bad design to make one class have multiple callbacks and do multiple things this way

This does mean each callback is isolated in it's own class, but IMHO thats fine (Laravel does this for lots of small things, such as listeners, jobs, notifications and the like.

I think for me personally, the best solution would be to be able to do this

Geo::callback('myCallback', myCallableOrClassNameHere)

This would be very similar to the way you bind things to Laravel's container from your service provider, or the way you add driver's to a manager.

[yak0d3]

I'm sure we do agree that the Laravel way is not always dependable, and Laravel have lots of classes where it uses interfaces such in artisan commands. I also have took into consideration that the callbacks would need arguments sometimes and those will all be available in the constructor.

I also agree that most applications won't need that many callbacks, but the same goes for artisan commands, yet having the feature will not hurt the code, in fact, it is a nice feature to be able to register all callbacks in a single path. We do have a similar concept in this package already, which is adding the 'or' prefix to callbacks' names, which is not necessary but it's there because it makes the code more readable, and that's good. I think we should always take into consideration rare use cases as well, as long as it does not hurt the code there is nothing to fear. It might in the contrary hurt the developers who face these rare use cases that we are ignoring.

To illustrate what i am saying, take a look at the Symfony Routing Component, it allows several ways to define Routes, and one of them is the annotations routes, which is a very very bad practice because it adds an extra layer of checking the annotations. This example is not just limited to this component, Symfony, which is one of the leading frameworks in the PHP world, always offers multiple options and tells you what the best practices are.

About the third point, i apologize but i am not sure if i do understand it, because callbacks classes do only need to implement a handle method.

Now for the last point: if it is a matter of naming that looks really nice. Otherwise if your meant that we should accept both callables and callbacks classes, that is exactly what this issue is about and seeing that you agree to having them both makes me feel more confident about working on this.

I hope i did not forget anything, i appreciate your comment 'cause it sincerely made me question the feature more and it gave me a few ideas on how to improve it.