Handling setters for traits

[djmattyg007]

I've got a couple of questions regarding how traits are handled.

Firstly, the Aura\Di\Factory class checks to see whether or not traits are available by calling function_exists('class_uses'), but the tests all seem to check which version of PHP is being used. Shouldn't they be using the same check? If you agree, which one would you want to standardise on?

Secondly, if you have a class that uses a trait, and that trait itself uses a second trait, calling class_uses() on the class won't include the second trait in the return array. This can result in some code duplication when initialising the DI container (see https://github.com/djmattyg007/WebFramework/blob/master/src/app/framework/di.php for an example; DatabaseCollection uses Database, so I don't think I should need to define DatabaseCollection as well).

Although I haven't yet tested it myself, by the looks of the comments on the relevant page from the PHP documentation, it wouldn't be hard to capture all child traits. http://www.php.net/manual/en/function.class-uses.php

I would be happy to submit a pull request that adds this functionality if you have no plans or no time to implement it yourself :) let me know what you think.

[harikt]

I've got a couple of questions regarding how traits are handled.

Great. I did looked the classes in v1 and v2 and I got the answer, we are talking about v2 here. It will be nice if we could point the version from now on in issues I guess.

Firstly, the Aura\Di\Factory class checks to see whether or not traits are available by calling function_exists('class_uses'), but the tests all seem to check which version of PHP is being used. Shouldn't they be using the same check? If you agree, which one would you want to standardise on?

Hm, that is a nice question. Technically both are correct. So I don't know the real answer and thank you for the detailed look into this.

Secondly, if you have a class that uses a trait, and that trait itself uses a second trait, calling class_uses() on the class won't include the second trait in the return array. This can result in some code duplication when initialising the DI container (see https://github.com/djmattyg007/WebFramework/blob/master/src/app/framework/di.php for an example; DatabaseCollection uses Database, so I don't think I should need to define DatabaseCollection as well).

The classes he is talking is over

https://github.com/djmattyg007/WebFramework/blob/master/src/app/framework/Core/Entity/Database.php
https://github.com/djmattyg007/WebFramework/blob/master/src/app/framework/Core/Entity/DatabaseCollection.php

I did spend some time to figure out. @pmjones may be this will help you to get into the file easily.

Although I haven't yet tested it myself, by the looks of the comments on the relevant page from the PHP documentation, it wouldn't be hard to capture all child traits. http://www.php.net/manual/en/function.class-uses.php

This is a special case I am seeing, especially using traits inside traits. Not sure whether this is the right way also. ( Let the right / wrong be there )

In that case I wonder whether it is good to use something like get_class_methods(<classname>)
http://www.php.net/manual/en/function.get-class-methods.php inside https://github.com/auraphp/Aura.Di/blob/develop-2/src/Factory.php#L419

I would be happy to submit a pull request that adds this functionality if you have no plans or no time to implement it yourself :) let me know what you think.

You can always open a PR. No issues, but merging yes @pmjones have the full right whether it is in or not. Else you need to wait for him :) .

Thank you

NB : Next time link to relevant files will help to find it easily.

[pmjones]

Firstly, the Aura\Di\Factory class checks to see whether or not traits are available by calling function_exists('class_uses'), but the tests all seem to check which version of PHP is being used. Shouldn't they be using the same check? If you agree, which one would you want to standardise on?

Good catch. They should all use the function_exists() variation. (I'll respond to the rest later.)

[pmjones]

by the looks of the comments on the relevant page from the PHP documentation, it wouldn't be hard to capture all child traits. http://www.php.net/manual/en/function.class-uses.php

I would be happy to submit a pull request that adds this functionality if you have no plans or no time to implement it yourself

@djmattyg007 Yes, another good catch, PR would be very welcome here. Thanks!

[djmattyg007]

I'll try and have a pull request submitted in the next few days :)

[pmjones]

At your convenience, sir.

[djmattyg007]

I have a similar question regarding constructors in interfaces. Currently, it's not possible to specify default parameters for constructors defined in an interface. I'm not sure if this functionality is viable or not, not in terms of implementation, but in terms of correctness.

Let's say you could have a class which implements more than one interface, and both of those interfaces define constructors that happen to be compatible with one another. You then specify default parameters for the constructors in both of these interfaces. When you grab this class out of the container, it won't be entirely clear which value you'll get for each parameter in the instantiated class.

If you had an understanding of how the container searched implemented interfaces for default parameters, and you knew the order in which interfaces were returned from ReflectionClass::getInterfaces() (see http://www.php.net/manual/en/reflectionclass.getinterfaces.php), you could work out exactly what each parameter's value would be. However, this is reasonably complicated.

I think it's worth pointing out that this is a problem with setters as well.

How do you think this should be handled?

[pmjones]

Currently, it's not possible to specify default parameters for constructors defined in an interface. I'm not sure if this functionality is viable or not, not in terms of implementation, but in terms of correctness.

Personally, I think it is incorrect for an interface to specify a constructor. For that reason I think honoring constructor params defined on a interface is not something we should pursue.

[pmjones]

Otherwise, this issue appears to be fixed as part of #45 . Thanks!

[djmattyg007]

Fair enough. Glad I could help :)