Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ditch EventInterface #77

Closed
jenkoian opened this issue Nov 21, 2016 · 5 comments
Closed

Ditch EventInterface #77

jenkoian opened this issue Nov 21, 2016 · 5 comments

Comments

@jenkoian
Copy link

I like to wrap dependencies in my own interfaces and use adapters so as not to be too coupled to a library, useful if, for example, I want to change from one library to another. I could do this with this library too, except for events as they rely on implementing EventInterface, thus meaning my event classes are coupled to this library.

So I was wondering if you'd consider ditching the interfaces on events? I guess the interface is useful to ensure the presence of a getName() method but wondering if we could get around that by a method exists check, perhaps falling back to the class name?

I can PR this if you are happy to do so. Tactician took a similar approach for commands a while ago too if it helps to understand the issue I'm likely horribly trying to describe thephpleague/tactician#43.

I'd understand if you want to keep the interface, but am interested in your thoughts none the less :)

@Zegnat
Copy link

Zegnat commented Nov 25, 2016

I’m not sure if this makes sense considering #70. Or at least I would say a choice needs to be made between either dropping interfaces or standardising against the proposed Event Manager standard and taking their interfaces.

@frankdejonge
Copy link
Member

@jenkoian I'm in favour of this. Sometimes when I feel like it I rewrite these packages and in many of the event implementation I've done besides this I've removed the need for an event interface. I'm thinking of pivotting this more into an domain event lib, though I also know there's a need for a more framework-ish event lib. Perhaps it can provide both, perhaps they can be two separate packages, I'm not sure yet. I'm open to suggestions.

@frankdejonge
Copy link
Member

Ok, so I make a PR for this #84

@binary-data
Copy link

Hi, any update on this?

Static analyzers like PHPStan produce "Call to an undefined method" warnings on custom event class methods and clutter the output.

    public function handle(EventInterface $event): void
    {
        // Call to an undefined method warning here
        $event->getValue();
       ...
    }

Type annotations are helpful for IDE but static analyzers treat them differently.

It would be nice to provide my own class annotation so IDE and other tools see methods correctly.

@frankdejonge
Copy link
Member

Version 3 (released soon) has no event interface.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants