2012/9/26 Tim Starling tstarling@wikimedia.org:
On 26/09/12 03:54, Tyler Romeo wrote:
This looks pretty interesting. Is there a reason we don't just put this in the core?
It has about 50 lines of useful code wrapped in 1600 lines of abstraction. I don't think it is the sort of style we want in the core.
I am sorry, but I find this comment a bit harsh, and just wanted to deliver some data on that. Maybe it was meant as a hyperbole, in which case I apologize for not treating it as such.
The extension has, altogether 2846 total lines of code in 24 files, of which 332 lines are blank, 1328 lines are comments, and 1186 are physical lines of code.
Of the latter 1186, 641 are tests. I find that commendable. (That leaves us with 1663 total lines of code, which are probably the 1600 from the original comment)
Another 148 physical lines go to initializing the extension and internationalization.
Remain 402 physical lines of code.
Now, one might discuss the suitability of using interfaces in PHP, but we have that in core: IDBAccessObject, ICacheHelper, IDeviceDetector, and a few others. Not many, but they exist. Anyway, the two interface classes account for only 18 physical lines of code anyway. The only 18 lines a user of the extension needs to care about.
One might discuss the suitability of using a class hierarchy to represent different DiffOps. But then again, that is the same class design as in the DairikiDiff engine, included in core as well.
There sure are other design choices that can be discussed. But the picture painted above was exaggerated, and I merely wanted to add some data to it.
Cheers, Denny
P.S.: terminology: "total lines of code" - lines in all code files, "physical lines of code" every line that has at least one non-comment or non-whitespace character. All numbers according to cloc.