[Routing] Allow easy extension of YamlFileLoader #8551

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
3 participants
Contributor

xphere commented Jul 23, 2013

Q A
Bug fix? no
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets N/A
License MIT
Doc PR N/A

I often found myself extending YamlFileLoader to add custom routing options, and I think it's quite hard to avoid duplicating the whole class. Validation of routes is a must, and I'd like to extend the default one with more keys, but this can't be done because declaration is private static and static binding is used. I think it would be nice to have an easy way to extend from this class.

Some solutions come to mind:

  • Change $availableKeys visibility to protected and use late static binding instead of self::$availableKeys.
    This way is easier to replace with new keys, but harder to add to current ones.
  • Extract keys to a protected method, say getAvailableKeys.
    Allows for easy replacement and extension, by calling parent::getAvailableKeys and mix with the new ones.
  • Create a Symfony/Config object in a protected method, and let derived classes to extend it.
    This brings even more power (validation and conversion) but couples Routing to Config components for a little profit.

I attach the code for the second solution (extraction to protected method).
This is an easy patch, but I would love to hear some opinions in the subject.

+ *
+ * @return array
+ */
+ protected function getAvailableKeys()
@entering

entering Jul 23, 2013

Contributor

static method? and then static::getAvailableKeys()

@xphere

xphere Jul 24, 2013

Contributor

Sorry if I'm missing something, but I can't see the benefits of that. Could you elaborate, please? I think a non-static method serves better for extending. I can imagine, for example, multiple instances of this class behaving different based on some construction parameters.

@entering

entering Jul 24, 2013

Contributor

"I can imagine, for example, multiple instances of this class behaving different based on some construction parameters."

Good point, commented because you were passing a private static var to a non static method.

Owner

fabpot commented Sep 27, 2013

The way to add custom routing options is to actually add them under the existing options item.

@fabpot fabpot closed this Sep 27, 2013

@xphere xphere deleted the xphere-forks:easy-yaml-loader-extension branch Dec 23, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment