Numerous places with duplicate code from core vBulletin

[kmike]

I went to check why some of our attachment changes didn't work in VaultWiki 2.5.7 and was unpleasantly surprised to found that VaultWiki uses a lot of vBulletin code, but not in the shared manner - the vB code is just copied to a VaultWiki script or class. For example, the discussion code is actually 95% showthread code. Granted, to make the discussions appear in a tab you indeed have to rip out the main code from showthread, it really couldn't be helped.

But the article postbit class (vB_WikiPost_Factory), why it's defined on its own, why doesn't it inherit from vB_Postbit_Post? Probably a dozen of vB_Postbit_Post class functions such as process_attachments() (my main gripe) had to be duplicated because of that. There had to be a very important reason behind this decision - what was it?

I haven't checked version 3.0, does it follow the same route?

[pegasus]

Version 3 is going to follow the same route. The reason this was done originally was because, like you mentioned, a lot of the main code was needed, but a lot needed to be changed in order to do what we wanted it to do. Granted, in some cases the classes COULD have inherited from existing classes, but in most cases, essentially EVERY function was modified in some way.

Even in the case where some weren't, we decided it would be better for memory management to include the 1 or 2 duplicate functions in a NEW class, rather than load both the PARENT AND CHILD classes during run-time (the difference between including 1 file or 2 files).

[kmike]

IMHO it was the most unwise decision. Now you have to track changes to the corresponding core vBulletin files in each new vB patch and apply them to your copied code. If you don't, the copied code may miss some fixes and as a result, the forum functionality provided by the copied code may perform differently in the wiki area than in the forum area.

And subsequently we have to apply our custom changes to your code as well.

[kmike]

Forgot to add that your argument about memory savings from having a different class instantiated instead of instantiating a child of existing class is wrong on so many levels. Even if these savings did exist, they'd be absolutely insignificant unless the class had a thousand or more instances instantiated at the same moment of time.

[pegasus]

IMHO it was the most unwise decision. Now you have to track changes to the corresponding core vBulletin files in each new vB patch and apply them to your copied code. If you don't, the copied code may miss some fixes and as a result, the forum functionality provided by the copied code may perform differently in the wiki area than in the forum area.

This is not a refusal to inherit some classes out of stubbornness. This is just something that cannot really be improved upon. In many cases, we tend to fix things before vBulletin - there were many parser inconsistencies in vB 3 that performed differently in the wiki parser, because we fixed them and vBulletin didn't for a while (e.g. problems with capitalization in BB-Code tags when used with NOPARSE).

As I said, almost every method was modified in many of these classes. The wiki postbit factory is almost completely unique and inheriting from the standard postbit factory would be pointless. The only class that COULD be changed into a child is the wiki BB-Code parser, but the benefits would be minimal - because only a few methods have not been modified.

This is an issue that has been revisited several times over the course of VaultWiki's development.

Here are the improvements that should be made: http://www.vaultwiki.org/issues/1265/