Painfully missing polymorphy


#1

Hi Jules,

while trying to override some functions in a customized subclass of KnownPluginList, I observed that the functions in my subclass never got called. This first confused me, because I’m coming from an OO background where polymorphy is always on (in C++ terms: everything is virtual, always). Unfortunately I learned that C++ does not allow overriding of non-virtual functions.

I see the performance argument, especially for audio processing, however, for the more high-level and administrative classes in Juce, I would rather see at least some fundamental functions be declared as virtual, so we can modify their behavior in our own derived classes.

The reason I stumbled across this was that I want to exclude certain filename patterns in plugin names from being scanned (the plugin should not scan itself, as that would cause a mess). Therefore I re-implemented isListingUpToDate() and scanAndAddFile() in my derived class, such that it checks the filename and calls the superclass method else.

As I do not want to modify the library code (by inserting “virtual” keywords here and there), the only way to deal with this is probably to copy-paste the entire KnownPluginList class (or the PluginDirectoryScanner)?

I would also like to suggest that the “Hidden virtual function” compiler warning be enabled, and all 306 instances of this fixed in the library (by adding virtual keyword in derived classes too). This way one would be warned when re-implementing functions without knowing they are not virtual in the base class.

Any alternative to this?

Thanks again for the wonderful library. I am getting along with it greatly.


#2

I didn’t make the method virtual because I don’t want anyone to override it. To try to add functionality to a class like KnownPluginList by overriding a big, complicated function like scanAndAddFile would be a total mess. Your subclass’s implementation would have to be very tightly coupled to the exact internal behaviour of the parent, so any changes to the parent class could break it.

What are you actually trying to do?


#3

I’m trying to exclude certain files from scanning, as the plugin doing this must not scan itself:

bool MyPluginList::isListingUpToDate (const String& possiblePluginFileOrIdentifier) { if ( possiblePluginFileOrIdentifier.contains ("MyPluginName") ) return true; else return KnownPluginList::isListingUpToDate ( possiblePluginFileOrIdentifier ); }

Attempted the same with scanAndAddFile, that is, just added a guard clause in the derived class.

I agree that a class requires to be designed from the beginning to be extensible. I consider my attempt a hack anyway, albeit it’s still more manageable than hacking the library code.

It would be great, if KnownPluginList and/or PluginDirectoryScanner had a provision for filtering plugins by filename (so they never get scanned), and filter plugins by their PluginDescription (in my case: include instruments only). I could remove unwanted types from the list later, but that’s a bit backwards.

Andre


#4

Well, adding a virtual method “bool shouldScanPlugin (plugin info)” would make much more sense than letting people override the class’s core methods. And of course the cleanest way (with slightly more typing) would be to have a class called “PluginFilter” (or something) that contains this function, and you’d create one of those to give to the knownpluginlist, without overriding it at all.


#5

Yes, the shouldScanPlugin( ) function would be a very helpful enhancement!

I didn’t mean this topic to suggest that the mentioned classes, or the library as such should allow overriding in general. It is just that I was quite surprised to see how restrictive class extension is in C++. Admittedly, I am spoiled by the convenience of other OO languages that I got used to over the years. Now that I know of this limitation, I will certainly learn to get along with it.

The point I was trying to make is that the compiler does not issue a warning when one attempts to override a non-virtual (warning is off by default). If I turn this on, it shows 300+ occurences in the library, so it is effectively useless.

Would it make sense to fix these warnings in the library and turn the compiler warning on by default? Or am I missing something important?


#6

It’s not restrictive enough, IMHO! I wish it had an equivalent of ‘final’ in java, which I could use to mark some classes as not suitable for overriding.

It is a pretty useless warning, because there are many cases where you want a class to inherit in a non-virtual way, because it just needs the base class’s features, but won’t ever be used polymorphically, but there’s no way for the compiler to know whether you’ve done this deliberately or whether it’s a genuine mistake. I think all the warnings in the library will just places where I’ve done it deliberately - but if you spot any real ones amongst the false alarms, let me know!