Feature/event restructure

[sagikazarmark]

Solves #4

[sagikazarmark]

Or an alternative to this: implement all current listeners as middlewares. But rather not. :P

[rosstuck]

I love me some middleware but not even I'd bother implementing all of these as middleware. :laughing:

In general, I like it. Seems like to cleans up the codebase and reduces the number of concepts a fair bit!

I think you could get rid of the abstract ConsumerAware entirely, it barely saves any code and you're actually working around it for the Wait constructor anyways. A public setter for a dependency is sub-optimal, $this->consumer->shutdown is just as short and an internal property is hardly worth the bother of a base class. :) So, I'd say chunk that part.

Otherwise, :+1: Nice work!

[sagikazarmark]

Regarding the constructor thing: I am thinking about two things:

• Make the Consumer a constructor parameter in every listener
• Have a setConsumer method

I think having a setter is better otherwise you would have to check the type every time shutdown is called.

Both have pros and cons. Haven't decided which is better yet. Any thoughts about this?

Also, yea, I agree, I am thinking about removing the base class. However the upper problem still needs to be solved. If we stick to the setter solution, it should remain.

[rosstuck]

I think it'd be better to have the Consumer as a constructor parameter for each one. It gives each class complete freedom on that part, while still abiding by a really simple API.

I'm not sure what typecheck you mean? The only one I see now is the null check but if it's a required parameter of the constructor, then that shouldn't be necessary. If a class does make it optional (for some bizarre reason), then yeah, they have to check but that's no biggy.

[sagikazarmark]

@rosstuck can you please check this out? Especially this part. Isn't it a bit vague? (Not just the docs, but the functionality as well)

[sagikazarmark]

See #6

[rosstuck]

For the docs example, maybe break them up per plugin. Here you can drop almost everything before the "// execute maximum of 10 commands" comment, I think.

Otherwise, I think it makes sense, though I'd use a separate eventEmitter, not the eventMiddleware for clarity's sake. That's just me though.

[sagikazarmark]

Yeah, breaking it up is a good idea.

The reason I like this EmitterTrait that exposes emitter functions is that in a DI context you might autoresolve the emitter when resolving the middleware, thus you might not even have direct imteraction with the emitter. That said, you can pass an emitter in the ctor if you like that better.

[rosstuck]

I really loathe autoresolving DI so that might be the difference in opinion. :)