👎 from me. Standalone extension manager should not be extended and docs should be corrected not to suggest that.

Rationale?

@Xerkus I disagree. In this case, extension will allow users tonot need a container implementation in order to provide more extensions; they can instead extend the standalone implementation to add theirs in.

@Synchro — I'm re-opening and will review.

I still oppose extending the class. It have numerous downsides.

• extended class will not be updated along with baseStandaloneExtensionManager if it copied the property.
• extended class that is changing behavior ofStandaloneExtensionManager should be separate implementation anyway as it will be a coupling of unrelated responsibilities.
• extending limits our ability to change implementation details at will, which is an additional maintenance burden. Not as much of a problem in this simple isolated case but it is there.

I see only two valid options here and none of them is to extend:

• introduce methods to configure extensions property to theStandaloneExtensionManager and forbid extension
• extend functionality via decorator implementing the interface, as I was saying in the related Issue:

finalclass MyExtensionManagerimplements ExtensionManagerInterface{private$decoratedManager;private$extensions = ['Media\Entry' =MyApp\RSS\Media\Entry::class,    ];publicfunction__construct(ExtensionManagerInterface$em)    {$this->decoratedManager =$em;    }/**     * Do we have the extension?     *     * @param  string $extension     * @return bool     */publicfunctionhas($extension)    {return (array_key_exists($extension,$this->extensions) ||$this->decoratedManager->has($extension));    }/**     * Retrieve the extension     *     * @param  string $extension     * @return mixed     */publicfunctionget($extension)    {if (array_key_exists($extension,$this->extensions)) {$class =$this->extensions[$extension];returnnew$class();        }return$this->decoratedManager->get($extension);    }}

And of course there is always configurable container implementation for more complex needs.

All this seems to be massive overkill - something that calls itself a plugin manager singularly fails to do anything matching that description - needing to write a convoluted decorator at all smells like a workaround for an inflexible base class. If a provided extension manager doesn't manage extensions, what is it for? A simpler alternative to extend/decorate would be to add an "addExtension" method, so I could say:

$m = new StandalonePluginManager;$m->addExtension('Media/Entry', 'MyApp\RSS\Media\Entry');

I'm using a framework because it helps me do things with less effort; I want to add oneextension, but I have zero interest in writing anextension manager - that's a role I would expect the framework to fulfil.

Extension manager is a contract established by the interface. Responsibility of the contract is to manage extensions for zend-feed, ie create, configure and provide.
StandaloneExtensionManager is a concrete implementation that is in no way obligated to provide anything beside said contract. Its responsibility is to make zend-feed work standalone without external dependency and it does exactly that.
NoopExtensionManager, for example, would do nothing, provide no extensions at all while still being a valid concrete implementation.

If you need flexible configurable extension manager, that role is filled byExtensionPluginManager. It is already written and supported.

As I said in previous comment, I will support expanding standalone responsibilities by making it configurable. Feel free to open PR for that.

Can you also provide same change for Writer?

OK. I've removedfinal. Are you happy with the general approach?

I've noticed that the code style of the extension list is quite different in each. In Reader:

    'Atom\Entry'            => 'Zend\Feed\Reader\Extension\Atom\Entry',

in Writer:

    'Atom\Renderer\Feed'           => Extension\Atom\Renderer\Feed::class,

They are consistent within themselves, but not with each other. It makes writing tests easier using the string style as I don't have to provide a mocked extension class - I know that older versions used the literal style, so I assume there's some history to that.

I noticed that the Writer doesn't have a test file for this, so I'm adding one.

::class pseudoconstant is preferred. It is a special case, it does not require class to actually exist.
What it does is resolves class using namespace and local imports, returns string.
\SomeClass::class and'SomeClass' are exactly the same, except static analysis will warn of non-existent class in former and won't in latter.

Hm. Looks like it fails to find classes using pseudoconstants:

Fatal error: Class 'Zend\Feed\Reader\Zend\Feed\Reader\Extension\Podcast\Feed' not found in /home/travis/build/zendframework/zend-feed/src/Reader/StandaloneExtensionManager.php on line 50

I will switch back to strings.

Uh, I could always just un-break the fully namespaced names...

Can you also update docs? For this change only

Fixing test data provider...

This check for Writer extensions makes little sense

How should it be ported to standalone extension manager?

All passing now. That OK?

Thanks,@Synchro