Proposed update to IdiormString (PHP > 5.3)

[tag]

I've written a simplified IdiormString class, that runs 30–40% faster than the current version (based on local speed tests).

See the implementation at tag/idiorm@b92d651

• Requires PHP > 5.3 because of the closure
• Removes need for IdiormStringException (wasn't necessary anyway, as method works just fine in a "bad" context).
• Makes IdiormString static only, so IdiormString::value($subject)->replace( ... ) access no longer usable. (This approach not used by Idiorm anyway.)
• Adds additional test (corrected at tag/idiorm@bb916dbd34)
• Removes about 80 lines of code (net) from Idiorm

Much of the speed gain and simplification comes from not creating an instance. It seems the only reason an instance was created was to get around using external variables in an callable, an outcome more easily accomplished with a closure.

With only one static function, IdiormString might not even need to be a separate class, and the method content can be merged into ORM, unrolled into the if block where it's called (line 300), reducing overhead even further.

Even if this isn't implemented, the $re_valid check in ::_str_replace_outside_quotes() can be removed, as the function works just fine on "bad context" strings without it. This also means IdiormStringException can be removed too, as it would never be thrown.

[treffynnon]

I already have a 5.3 version of this code waiting in the wings in my local version of this code. Obviously the reason it creates an instance at the moment is to be 5.2 compatible.

Thanks for the code. I am not sure I like the validation of the string being removed from it though.

Anyway I will review both versions and see where we get to.

[tag]

Glad you're on top of it. Didn't know you had a 5.3 version ready. The second commit "further unrolling", above (tag/idiorm@b0fb96b) could be made 5.2 compatible via create_function(), if desired.

I've been racking my brain trying to think of a valid SQL statement that would cause the validation check to fail, but would work when sent to the db, and can't come up with one. I've run several tests locally, and so far, even on invalid queries, the method performs fine without the validation check.

One of my reasons for wanting to remove the additional classes, both the exception and IdiormString, is to simplify eventual PSR-0 loading (milestone?), and keep Idiorm to a single file per the original vision (or at least as few as possible).

Anyway, as I said, I'm glad you're working on it.

[treffynnon]

A valid SQL statement will pass through the regex fine, but we are dealing with raw_queries which we have no guarantees will be built correctly by those implementing the code. Additionally I am not sure that the speed of this function is too much of a concern as it is only run when logging is turned on. Logging isn't run in production so it won't be hit.

I am pleased to be benefiting from all your code and help with both Paris and Idiorm though so thanks!

[tag]

Like I said, even when I pass broken queries, it seems to work fine without the validation check.

Speed when logging is only part of it. Aiming for a smaller file gives an infinitesimally small speedup in production too.

[treffynnon]

Ah OK I haven't actually tried to feed it broken queries without the validation check to be honest so you have investigated this further than I have so far. Thanks for the insights.