search

Jeroen-G / autowire

🔌 Autowire and configure using PHP 8 Attributes in Laravel.
MIT License
21 stars 3 forks source link

Allow for extending of Attributes #4

Closed bitwise-operators closed 2 years ago

bitwise-operators commented 2 years ago

This allows extending the base Autowire and Configure attributes while still retaining full functionality.

Jeroen-G commented 2 years ago

Curious: why would you want to extend the two?

bitwise-operators commented 2 years ago

Dependency inversion would be my main reason.

In a big project (where tooling like this is most worthwhile), having use JeroenG\Autowire\Attribute\Autowire; in every interface file becomes a maintenance nightmare if, for whatever reason, you have stopped updating this package by the time PHP 9 comes around, and whoever takes it over changes the namespace.

By extending the attribute and using that throughout the code, updating the extended class is enough if the base namespace changes.

Jeroen-G commented 2 years ago

I am going to sit on this for a while.

My reasoning: refactorings like you mentioned cost seconds with things tools like PHPstorm offer nowadays. I made those two classes final also to have the option to modify them in the future without having to worry about others having extended them and breaking their applications. So while I do appreciate the contribution, I am not merging (but also not closing) this now.

I maintain several packages now for years, and can assure you that I won't just hand over this package or remove it :)

bitwise-operators commented 2 years ago

First of all: Your package, your rules!

I was expecting the IDE argument. You don't even need an IDE, you could do it with some simple command-line commands.

However, you would end up with a huge git commit touching most if not all of your interfaces, while the interfaces themselves haven't changed, which violates SOLID principles. Which is why I prefer to use dependency inversion.

My solution here isn't even the best way to do it, but was the least amount of code to get it working.

A more robust way would be if your package supplies interfaces the attributes have to implement, and the user can provide his own attributes in the config if he wants.

I've created a second pull request, implementing that solution, you can find it here.

Jeroen-G commented 2 years ago

I like that one much more! I'm closing this in favour of your other PR! 👏