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

[suggest] Ignore methods without parameters from aware interfaces #4918

Closed
wants to merge 3 commits into
base: master
from

Conversation

Projects
None yet
4 participants
@tux-rampage
Contributor

tux-rampage commented Jul 31, 2013

Some aware interfaces in ZF2 define setters which do not expect any parameter.
These setters are not thought to be called by the dependency injector.

It doesn't make sense anyway to process interface methods from aware interfaces that doesn't have parameters. Even worse: Di could call a method that it should not invoke.

@Ocramius

This comment has been minimized.

Show comment
Hide comment
@Ocramius

Ocramius Jul 31, 2013

Member

@tux-rampage the idea seems ok - just wondering why there would be aware interfaces without params, but that's another problem...

Member

Ocramius commented Jul 31, 2013

@tux-rampage the idea seems ok - just wondering why there would be aware interfaces without params, but that's another problem...

@tux-rampage

This comment has been minimized.

Show comment
Hide comment
@tux-rampage

tux-rampage Aug 6, 2013

Contributor

@Ocramius I'm wondering about this, too. But it doesn't bother me much.

Some define the getter as well as the setter. This is a design/philosophy discussion, IMHO not worth debating to much about it. We should give ZF2 consumers this flexibility, no matter if it makes sense to a group of people or not. I guess it doesn't hurt anyone.

Contributor

tux-rampage commented Aug 6, 2013

@Ocramius I'm wondering about this, too. But it doesn't bother me much.

Some define the getter as well as the setter. This is a design/philosophy discussion, IMHO not worth debating to much about it. We should give ZF2 consumers this flexibility, no matter if it makes sense to a group of people or not. I guess it doesn't hurt anyone.

@samsonasik

View changes

Show outdated Hide outdated tests/ZendTest/Di/TestAsset/AwareClasses/A.php
@samsonasik

View changes

Show outdated Hide outdated tests/ZendTest/Di/TestAsset/AwareClasses/NoParamsAwareInterface.php
@samsonasik

View changes

Show outdated Hide outdated tests/ZendTest/Di/TestAsset/AwareClasses/B.php
@weierophinney

View changes

Show outdated Hide outdated library/Zend/Di/Definition/CompilerDefinition.php
@weierophinney

View changes

Show outdated Hide outdated library/Zend/Di/Definition/RuntimeDefinition.php
@weierophinney

This comment has been minimized.

Show comment
Hide comment
@weierophinney

weierophinney Aug 19, 2013

Member

@tux-rampage incorporate the feedback, and I'll review for merge.

Member

weierophinney commented Aug 19, 2013

@tux-rampage incorporate the feedback, and I'll review for merge.

@tux-rampage

This comment has been minimized.

Show comment
Hide comment
@tux-rampage

tux-rampage Aug 27, 2013

Contributor

@weierophinney I changed the code as requested.
But why so serious about explicit checks?

Contributor

tux-rampage commented Aug 27, 2013

@weierophinney I changed the code as requested.
But why so serious about explicit checks?

@weierophinney

This comment has been minimized.

Show comment
Hide comment
@weierophinney

weierophinney Aug 28, 2013

Member

@tux-rampage Because in this context, we could have a situation of testing 0 == '_construct', which would evaluate to true. :)

Member

weierophinney commented Aug 28, 2013

@tux-rampage Because in this context, we could have a situation of testing 0 == '_construct', which would evaluate to true. :)

weierophinney added a commit that referenced this pull request Aug 28, 2013

Merge pull request #4918 from tux-rampage/di-fixes
[suggest] Ignore methods without parameters from aware interfaces

weierophinney added a commit that referenced this pull request Aug 28, 2013

[#4918] CS fixes
- EOF markers

weierophinney added a commit that referenced this pull request Aug 28, 2013

@ghost ghost assigned weierophinney Aug 28, 2013

weierophinney added a commit to zendframework/zend-di that referenced this pull request May 15, 2015

Merge pull request zendframework/zendframework#4918 from tux-rampage/…
…di-fixes

[suggest] Ignore methods without parameters from aware interfaces

weierophinney added a commit to zendframework/zend-di that referenced this pull request May 15, 2015

weierophinney added a commit to zendframework/zend-di that referenced this pull request May 15, 2015

weierophinney added a commit to zendframework/zend-di that referenced this pull request May 15, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment