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

Some improvements #6

Closed
akkie opened this issue Sep 13, 2012 · 5 comments
Closed

Some improvements #6

akkie opened this issue Sep 13, 2012 · 5 comments

Comments

@akkie
Copy link

akkie commented Sep 13, 2012

Hi,

here is a list of some suggested improvements for the lib.

Cache:

  • Only APC is supported(Why not using Doctrine\Common\Cache)

  • I've observed a strange caching behaviour. It seems that the lib caches sometimes an inconsistent state. This is reproducible with the following steps:

    1. Create correct bindings
    2. Remove Inject annotation
    3. Clear cache
    4. Reload(must show an error - Uncaught exception 'Ray\Di\Exception\NotBinded' with message 'Bind not found. argument #0...)
    5. Add Inject annotation
    6. Relaod(shows still the exception)

    If I clear the cache and reload the page then it works as expected.

Inject Annotation

  • The Inject annotation cannot be loaded with an own autoloader implementation. It seems that the doctrine annotation implementation relies on the doctrine autoloader. As a workaround I must include the annotation by hand.

    This is the error:

    Fatal error: Uncaught exception 'Doctrine\Common\Annotations\AnnotationException' with message '[Semantical Error] The annotation "@ray\Di\Di\Inject" in class util\affiliate\Broker does not exist, or could not be auto-loaded

  • If the Inject annotation is missing, the lib throws a NotBinded exception. I think in this case it should throw an exception that the annotation is missing.

    This is the error:

    Fatal error: Uncaught exception 'Ray\Di\Exception\NotBinded' with message 'Bind not found. argument #0($userDAO) in

  • The FQN @Ray\Di\Di\Inject does not work. Only the short form Inject does work as definition. I rather thought that doctrine supports the fully FQN syntax.

Binding

  • It isn't possible to declare the binding with a leading back slash:
<?php

// Doesn't work
$this->bind('\model\daos\UserDAO')->to('\model\daos\DbUserDAO');

// Does work
$this->bind('model\daos\UserDAO')->to('\model\daos\DbUserDAO');

This is the error:

Uncaught exception 'Ray\Di\Exception\Binding' with message 'model\daos\UserDAO:*'

@koriym
Copy link
Member

koriym commented Sep 13, 2012

Cache

  • APC, the primary reason is the speed, But more universal cache adapter is also Ok, ApcConfing / ApcInjector is designed to be easily converted. (Config doesn't include apc function, ApcConfig does.)
  • You can't change annotation setting with cache on setting. cache clear always needed.

Inject Annotation

  • @Inject / @Named require FQN use statements. and Doctrine annotation autoloader (or load manually), this works.
use Ray\Di\Di\Inject;
use Doctrine\Common\Annotations\AnnotationRegistry;

AnnotationRegistry::registerAutoloadNamespace('Ray\Di\Di\\', 'path/to/vendor/Ray/Di/src/');
  • Note: @Inject without FQN use statement works in cache off mode, but doesn't work in cache mode. This confusion/problem is caused by I use lower level Doctrine DocParser instead of Doctrine AnnotationReader to avoid explicit use statement every time (but I now think its bad idea, should be changed later.)

@koriym
Copy link
Member

koriym commented Sep 24, 2012

Inject Annotation is fixed in koriym@4eeddc8#src/Ray

I opened #11 for Binding

@koriym
Copy link
Member

koriym commented Sep 28, 2012

biding fix with 0a3e26b

@koriym
Copy link
Member

koriym commented Sep 28, 2012

@akkie thanks for the suggestion.

@koriym koriym closed this as completed Sep 28, 2012
@koriym koriym reopened this Jan 30, 2013
@koriym
Copy link
Member

koriym commented Jan 30, 2013

@akkie Doctrine\Common\Cache is supported now in 1.0.0-beta4 2716f30

@koriym koriym closed this as completed Jan 30, 2013
koriym added a commit that referenced this issue May 7, 2015
#5 fix concrete class type-hint issue
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

2 participants