PRS-2 reformatting of code

[abolabo]

just added phpdoc comments and added changes related to readability of code

[antonkomarev]

It's so much different types of changes you've done within one PR. Very hard to review it.

[antonkomarev]

Why? 😭

[abolabo]

oh.. sorry.. mistake.. please revert

[Kern046]

Hi ! Thanks for your interest in this library and for your contribution ! I'm glad to have such interactions with the community !

However, I removed on purpose the PHPdoc blocks in the methods, as I find it quite useless since PHP7 introduces strict typing which serves also as documentation. I'm not very fond of putting it back.

I would've liked to discuss it first within an opened issue instead debating already-done work ! The fault is mine as I didn't set a proper contributing policy for this repo. I will write it soon enough.

If you have further modifications to propose, I suggest you open an issue first, so we can discuss it without a chance to do unmerged work !

[Kern046]

It is not useful to set the Manager in PHPdoc if the interface is present. You can remove it :)

[abolabo]

hm.. i think you are not right. Phpdoc comments used by IDE such as PhpStorm to show relations between different code. All logic of code-inspectors based on phpdocumentor comments. So if you will run inspector it will show a lot of errors "unknown classes" etc.
Anyway relations of code is important thing if you decided to rename (refactor) some of class.
(Or in case when you call some class not directly, dynamically)

[Kern046]

Sorry I wasn't precise enough. I just said that you could do:

/** @var AttributeManagerInterface **/ and not mention `AttributeManager`` since the purpose to set interfaces instead of classes directly is to remove the dependency to a precise class.

I have no doubts on the utility of PHPdoc comments to declare properties type. It's just for methods where I suggest that PHP7 type-hinting is enough for modern IDEs

[abolabo]

oh, yes. But in this case IDE will not hint methods of some sibling class of interface.
I mean quick jump to class method from call by ide shortcut ( ctrl+click ). If i describe parameter only as interface class ide jumps into interface class, not child.
Anyway it's your code, your rules :-)

[antonkomarev]

There is no need to specify all siblings in DocBlock. Only interfaces is fine.
After redirecting in IDE to interface you can choose on what sibling you want to redirect further.

[antonkomarev]

It's good practice to have FQCN (Fully Qualified Class Names) in DocBlocks.

[Kern046]

@abolabo The matter in open-source is not whose code it is, but what is the best way to code this library, and I'm totally open to changes even the ones I wouldn't do myself if it is the best thing according to conventions etc...

Your PR is good, it is totally right to apply SOLID principles here. You did transform the class type-hint into interfaces to remove hard dependencies between classes and it is essential to allow overriding of the library components. To be coherent with this change, the PHPdoc must also be updated, but must not keep the old hard dependency to the classes such as AttributeManager.

In the meantime, you can remove the classes use statements which would become useless with the replacement by interfaces.

[Kern046]

I understand your concern about jumps in IDE but if someone overrides AttributeManager and set it as Abac dependency, it would be wrong that the IDE jumps to the native AttributeManager instead of the extended one. Jumping to the interface seems to be more coherent

[Kern046]

Is it really useful to say that this is the class constructor ?