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.
 

Sim

Well-known member
#5
Hey @Kirby how did you end up working around these limitations?

I've come across the same issue when trying to extend the functionality of the new XFMG Embedded Data functions - I need to extend the parent class, which I can't do because there are child classes.

Looks like I'm going to have to get a bit hacky.
 

Xon

Well-known member
#6
@Sim what I've been using is Traits to inject a common shared code base into various classes. Content Ratings heavily uses traits to inject reactions/rating support into arbitrary entities.

Report Queues & Stats for XF1 actually dynamically generates classes with a trait in response to the each controller it encounters. This in't possible in XF2, (because of the class extension system) but isn't required as I can extend just the bits I need.
 

Sim

Well-known member
#7
Turns out some minor code changes to the core would make my issues trivial to fix: https://xenforo.com/community/threads/xf2-code-change-request-getcharset.142167/

... the alternative is very very messy.

@Xon sure, I guess I could just extend each of the child classes and add a trait to each with the code that needs to be over-ridden, but it's still a bit of a nasty hack (in that I have to keep checking that the trait code I copy from the core hasn't changed with each new release of XF) which has much neater solutions with some minor work on the part of the devs.
 
Top