DCOM-2: Annotation and autoloading #503

Closed
doctrinebot opened this Issue Apr 20, 2010 · 6 comments

1 participant

@doctrinebot

Jira issue originally created by user chebba:

The problem is that we need to load all annnotation classes before we read annotations from the target class.
So we need to require_once them at target class source, or just before reading.
It's not cool.

The reason of this, is a fix of bug #77
http://www.doctrine-project.org/jira/browse/[DDC-77](http://www.doctrine-project.org/jira/browse/DDC-77)
and class_exists(..., false).

Solutions:
1. Filter DocBlockTags, and think that all other @foo string is annotations (don't check with class_exists)
2. Use class_exists(..., true) but supress warnings with @ and try/catch operators.

May be there are some another, better solutions.

@doctrinebot

Comment created by romanb:

The classexists check can maybe be removed altogether since is_subclassof is no longer used.

@doctrinebot

Comment created by merk:

Would removing the second parameter to class_exists solve the problem sufficiently?

I would thing you would have to leave the class_exists check in, otherwise it will try to parse annotations that do not resolve to class names?

@doctrinebot

Comment created by romanb:

The fundamental problem with classexists($blub) (which is the same as class_exists($blub, true) is that it basically *requires a class loader / autoloader that fails silently when a class file does not exist, which means it must use costly fileexists checks before loading *any** class. The Doctrine\Common\ClassLoader does (on purpose) not check for file existance. If the class loader is responsible for a particular class and it is requested to load it, failing to do so is (and should be) a fatal error.

We consider the fact that two responsibilities are mixed here, a) (auto)loading a class file and b) checking for the existance of a class (file), to be a design flaw in classexists / autoloading. It means that you are required to check for file existance in a class loader and fail silently if the file does not exist which is completely unnecessary in 99% of the cases. Compare how many times class_exists(..., true) is usually used in a single request and how many classes, in total, are usually loaded per request. So just for the case that some code might use an occasional class_exists(..., true) check you have to check each single file for existance before loading it (and you must fail silently, it might be a classexists check!).

Even more, in a case where you really only want to check for class (file) existance, but you dont actually want to load it, you can't! class_exists(..., true) when the class file exists results in loading the class file, even if its completely unnecessary.

We are planning to come up with a better approach that separates the 2 concerns.

This is related to DCOM-7.

@doctrinebot

Comment created by romanb:

The recommended approach for annotations by the way, (recommended by me), is to load them manually via a require call. The ORM does exactly this. If you rely on classexists(..., true) and silently failing autoloaders, this becomes costly considering how often classexists would be invoked while parsing annotations.

@doctrinebot

Issue was closed with resolution "Fixed"

@doctrinebot doctrinebot added this to the 2.0.0-BETA3 milestone Dec 6, 2015
@doctrinebot doctrinebot closed this Dec 6, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment