• This site uses cookies. By continuing to use this site, you are agreeing to our use of cookies. Learn more.

XF 2.0 Extending the parent of a class

Kirby

Well-known member
#1
Today I found myself in a situation where I would have needed to extend an abstract base class in order to implement some features for the widget system.

Unfortunately this does not work as it is currently not possible to extend any class that is a parent of another class.

Do others also have experienced this?
And if so: Did you find workarounds (apart from extending every childclass)?

What are the reasons (performance?) for implementing it this way?
 
Last edited:

Mike

XenForo developer
Staff member
#2
And if so: Did you find workarounds (apart from extending every childclass)?
There wouldn't really be one. That itself would be quite difficult to do.

What are the reasons (performance?) for implemented it this way?
Well the only way to resolve it would be to have every class essentially inherit from a non-existing base class (like add-ons). There certainly are some potential performance overheads, though probably more cognitive overheads...

Saying that, you probably could do it by overriding specific classes in the autoloader, though that will be somewhat hacky as well.
 

Kirby

Well-known member
#3
Thinking a bit more about this:
As it is implemented right now, the class hierarchy is being built at runtime and the final class name varies depending on the active class extensions.

So if there are 2 Add-ons (VendorA/AddOnA and VendorB/AddOnB) that both extend \XF\Spam\Cleaner the class hierarchy would look like this:

Code:
\VendorB\AddOnB\XF\Spam\Cleaner (in <rooot>/src/addons/VendorB/AddOnB/XF/Spam/Cleaner.php)
  \VendorA\AddOnB\XF\Spam\Cleaner alias \VendorB\AddOnB\XF\Spam\XFCP_Cleaner (in <rooot>/src/addons/VendorA/AddOnA/XF/Spam/Cleaner.php)
    \XF\Spam\Cleaner alias \VendorA\AddOnA\XF\Spam\XFCP_Cleaner (in <rooot>/src/XF/Spam/Cleaner.php)
In order for this to work, all code using \XF\Spam\Cleaner has to be specifically crafted to support this, eg.
PHP:
$class = $this->extendClass('XF\Spam\Cleaner');
$object = new $class($this);
If some code from an Add-on doesn't follow this principle and directly instantiates \XF\Spam\Cleaner none of the extended classes does get used which might cause various side-effects that could go unnoticed.

Do we really need dynamic class extensions (eg. \XF\Extension::addClassExtension() and \XF\Extension::removeClassExtensions())?

If we could got get rid of them, it might be possible to turn the whole Class Extension System upside down, so instead of having a variable final class name wo could instead change the name of the base class if it is extended:
Upon mutation of the class hierarchy (due to enabling/disabling class extensions, installing/uninstalling/enabling/disabling Add-ons) the system could statically build the class hierarchy and compile all necessary extensions into a cached file which does then get loaded at runtime.

For the above example this cached file would be
PHP:
namespace XF\Spam
{
    use XF\Entity\User;

    abstract class XFBC_Cleaner
    {
    }
}

namespace VendorA\ProductA\XF\Spam
{
    use XF\Entity\User;

    abstract class Cleaner extends \XF\Spam\XFBC_Cleaner
    {
    }
}

namespace VendorB\ProductB\XF\Spam
{
    use XF\Entity\User;

    class Cleaner extends \VendorA\ProductA\XF\Spam\Cleaner
    {
    }
}
and should then be aliased (eg. \class_alias('\VendorB\ProductB\XF\Spam\Cleaner', '\XF\Spam\Cleaner')) and loaded by the autoloader instead of <root>/src/XF/Spam/Cleaner.php

To me this seems to have several advantages over the current approach
  • No overhead for generating the class hierarchy at runtime
  • No cognitive overheads
  • Probably slightly faster loading
  • No need for specifically crafted code
  • Possibility to also extend abstract/parent classes (probaby all except \XF and \XF\App)
  • Safeguard against accidental instantiation of an extension class like \VendorA\ProductA\XF\Spam\Cleaner, which is currently possible (if the class was loaded before thus the aliases have been created)

What do others think about this, does it make sense?
 
Last edited:

Xon

Well-known member
#4
Do we really need dynamic class extensions (eg. \XF\Extension::addClassExtension() and \XF\Extension::removeClassExtensions())?
If the class has been loaded, removeClassExtensions is pointless. And if the class has been loaded all addClassExtension can do is to add another child class. Except most objects now life the life of the request, so most things will likely be already instantiated when addClassExtension can be called.

I did use the moral equivalent in XF1, of disabling extensions before loading some custom class to work-around an add-on's lack of feature flags. But the Finder system should help with that.