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

Convert paths into Mapping objects #49

Open
wants to merge 3 commits into
base: 1.x
Choose a base branch
from
Open

Convert paths into Mapping objects #49

wants to merge 3 commits into from

Conversation

guiwoda
Copy link
Contributor

@guiwoda guiwoda commented Jul 4, 2017

This PR adds a function that can convert an array of paths into an array of Mapping objects by reflecting on declared classes, just as the AnnotationDriver does.

The FluentDriver API is almost unchanged. To support receiving already-instanced Mapping objects, the addMapping methods had to be relaxed. Took the opportunity to refactor that relaxation to the lowest-level method, but this could be rolled back if BC matters.

We are debating between this and #48 😄

guiwoda and others added 2 commits July 4, 2017 18:40
This function can convert an array of paths into an array of Mapping objects by reflecting on declared classes, just as the AnnotationDriver does.
[ci skip] [skip ci]
@guiwoda guiwoda requested a review from patrickbrouwers July 4, 2017 21:43

$mappings = [];
$declared = get_declared_classes();
foreach ($declared as $className) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@patrickbrouwers moved inner loop outside the $paths loop, so it recursively iterates all paths, then iterates over declared classes once.

foreach ($declared as $className) {
$rc = new ReflectionClass($className);
$sourceFile = $rc->getFileName();
if ($sourceFile === false || !array_key_exists($sourceFile, $includedFiles)) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@patrickbrouwers switched to key check instead of in_array. Had to use the boolean check to avoid php native classes crashing the array_key_exists check.

@@ -36,6 +36,7 @@
"gedmo/doctrine-extensions": "^2.4"
},
"autoload": {
"files": ["src/helpers.php"],
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@patrickbrouwers there's no other way to load functions, right? I couldn't find documentation of namespaced functions in composer docs.
Oh, this probably breaks in php 5.5, so we should bump anyway.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Commited bump in here so travis doesn't fail on phpunit run.

{
$includedFiles = [];

foreach ($paths as $path) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@patrickbrouwers I'm pretty sure we should be able to replace this loop with another iterator, so that we iterate recursively over each path, instead of building the full iterator chain for each path in the array.

@eigan
Copy link
Member

eigan commented Mar 3, 2021

@guiwoda Any reason why this wasn't merged?

@eigan eigan changed the base branch from 1.1 to 1.x March 3, 2021 06:46
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

Successfully merging this pull request may close these issues.

3 participants