Doctrine Common
  1. Doctrine Common
  2. DCOM-13

Better flexibility when the Annotation is created by the Parser

    Details

    • Type: Improvement Improvement
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 2.0.0-BETA4
    • Component/s: Annotations
    • Labels:
      None

      Description

      The creation of the Annotation class is done at the end in Parser::Annotation(). It assumes that the Annotation class constructor takes an array of values. But if this is not the case, you are out of luck. So, I propose to move the logic of Annotation creation to is own method for better flexibility.

      1. patch
        0.9 kB
        Fabien Potencier

        Activity

        Hide
        Jonathan H. Wage added a comment -

        What is the reason your annotation class can't have a single argument array constructor?

        Show
        Jonathan H. Wage added a comment - What is the reason your annotation class can't have a single argument array constructor?
        Hide
        Jonathan H. Wage added a comment -

        Currently it is by design that Annotation classes take a single argument array constructor and the Reader and Parser are not designed for inheritance.

        Show
        Jonathan H. Wage added a comment - Currently it is by design that Annotation classes take a single argument array constructor and the Reader and Parser are not designed for inheritance.
        Hide
        Fabien Potencier added a comment -

        To sum up:

        • You force me to design my Annotation class is a certain way (for no obvious reason);
        • You don't want the Parser and Reader to be extensible.

        So, basically, you say that the Doctrine Annotation library is not designed to be used outside Doctrine? That's fine, but then I will need to create my own Annotation component, which makes me sad.

        Show
        Fabien Potencier added a comment - To sum up: You force me to design my Annotation class is a certain way (for no obvious reason); You don't want the Parser and Reader to be extensible. So, basically, you say that the Doctrine Annotation library is not designed to be used outside Doctrine? That's fine, but then I will need to create my own Annotation component, which makes me sad.
        Hide
        Roman S. Borschel added a comment -

        @"You force me to design my Annotation class is a certain way (for no obvious reason)"

        It is a contract of the implementation, nothing more, nothing less. An annotation class must have a single argument array constructor. Would you feel better if there were an interface with such a constructor? Any contract always "forces" you to design your class that implements that contract in a certain way.

        @"You don't want the Parser and Reader to be extensible."

        They simply are not designed to be inherited right now which simply means we don't care about backwards compatibility with regards to subclasses when changing these classes and we're not documenting and telling people it is a good idea to subclass them. And frankly, inhertiance just to swap out the annotation construction process is a bad idea. It reveals & couples the subclasses to all the details of the parent class even though all they actually want is exchange the construction process. Moreover you would right now need to extend both, the parser and and reader to swap out the construction process. At least the parser would then need to become an (optional) constructor argument of a reader in order not to need to subclass the reader. Inheritance => strong coupling => hard to maintain backwards compatibility.

        If there is strong desire to replace the annotation construction process, a factory (method/closure) approach would be much better.
        Can you maybe reveal your concrete use-case? That an annotation class must have a single-argument array constructor is not an unreasonable requirement and you're really exaggerating if you're saying that this simple requirement makes the annotation library unusable outside of Doctrine. That is ridiculous. The Symfony Validator component already uses it.

        Please try to be more constructive. Neither did you show concrete use-cases nor is your suggested solution a good idea as it is now because it requires 2 classes to be extended plus 1 constructor and 1 method to be overridden just to exchange the annotation construction process. This can surely be done simpler and will be done simpler if there is enough need for this enhancement.

        Show
        Roman S. Borschel added a comment - @"You force me to design my Annotation class is a certain way (for no obvious reason)" It is a contract of the implementation, nothing more, nothing less. An annotation class must have a single argument array constructor. Would you feel better if there were an interface with such a constructor? Any contract always "forces" you to design your class that implements that contract in a certain way. @"You don't want the Parser and Reader to be extensible." They simply are not designed to be inherited right now which simply means we don't care about backwards compatibility with regards to subclasses when changing these classes and we're not documenting and telling people it is a good idea to subclass them. And frankly, inhertiance just to swap out the annotation construction process is a bad idea. It reveals & couples the subclasses to all the details of the parent class even though all they actually want is exchange the construction process. Moreover you would right now need to extend both, the parser and and reader to swap out the construction process. At least the parser would then need to become an (optional) constructor argument of a reader in order not to need to subclass the reader. Inheritance => strong coupling => hard to maintain backwards compatibility. If there is strong desire to replace the annotation construction process, a factory (method/closure) approach would be much better. Can you maybe reveal your concrete use-case? That an annotation class must have a single-argument array constructor is not an unreasonable requirement and you're really exaggerating if you're saying that this simple requirement makes the annotation library unusable outside of Doctrine. That is ridiculous. The Symfony Validator component already uses it. Please try to be more constructive. Neither did you show concrete use-cases nor is your suggested solution a good idea as it is now because it requires 2 classes to be extended plus 1 constructor and 1 method to be overridden just to exchange the annotation construction process. This can surely be done simpler and will be done simpler if there is enough need for this enhancement.
        Show
        Jonathan H. Wage added a comment - This was fixed in http://github.com/doctrine/common/commit/66dad3e22205d812911adeb32b9f8bab8879d4b7

          People

          • Assignee:
            Roman S. Borschel
            Reporter:
            Fabien Potencier
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: