Skip to content

[Form] [Hackday] [2.7] Add a deprecation note about VirtualFormAwareIterator #12651

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.

Sign up for GitHub

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

Closed
wants to merge 3 commits into from
Closed

[Form] [Hackday] [2.7] Add a deprecation note about VirtualFormAwareIterator #12651

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
188e3ec
issue 12604
mrcmorales Nov 29, 2014
f1532fa
moved the notice to the constructor
mrcmorales Nov 29, 2014
7044489
php-cs-fixer
mrcmorales Nov 29, 2014
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions src/Symfony/Component/Form/Util/VirtualFormAwareIterator.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,15 @@
*/
class VirtualFormAwareIterator extends \IteratorIterator implements \RecursiveIterator
{
public function __construct(\Traversable $iterator)
{
parent::__construct($iterator);

if ('Symfony\Component\Form\Util\VirtualFormAwareIterator' === get_class()) {
trigger_error('class VirtualFormAwareIterator is deprecated since version 2.7 and will be removed in 3.0. Use InheritDataAwareIterator instead.', E_USER_DEPRECATED);
Copy link
Member

Choose a reason for hiding this comment

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

please move the notice in the constructor (but do not forget to call the parent)

Copy link
Member

Choose a reason for hiding this comment

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

We discoevered a better way to trigger the notice: below the namespace declaration, see https://github.com/symfony/symfony/pull/12739/files
(btw, I don't see the case for the if check, can you remove it or justify it please?)

Copy link

Choose a reason for hiding this comment

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

why not throwing the trigger_error in the constructor and not after the namespace declaration, like: https://github.com/symfony/symfony/pull/12739/files ?

Copy link
Member

Choose a reason for hiding this comment

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

Adding it after the namespace declaration means that it is only triggered once when the class is loaded and not every time an instance of it is created.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@nicolas-grekas The reason about if is because class InheritDataAwareIterator extends of this class

https://github.com/symfony/symfony/blob/2.7/src/Symfony/Component/Form/Util/InheritDataAwareIterator.php#L26

Copy link
Member

Choose a reason for hiding this comment

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

@nicolas-grekas putting it when loading the file does not trigger a warning each time the class is instantiated (and could trigger it when not needed). We need to trigger the error only when the deprecated feature is used (which may not even be at instantiation time if it is always instantiated for BC reasons)

}
}

/**
* {@inheritdoc}
*/
Expand Down