Josh wrote:
> (I know it's readability vs. speed, but in this case I'd rather
> have speed.)
That's easy for you to say: you don't have to maintain the code! :-)
Although its undeniable that the code could be faster, it's unlikely
that anyone is going to be calling .replace enough times in one day
to notice any significant difference.
I wouldn't argue against speed over style if we were talking about the
Stash dotop() method which is getting hammered continually. But in the
case of a single virtual method, any extra effort expended here is going
to have limited returns in all but the most extreme examples (e.g.
benchmarks).
> Also, this doesn't check to see if we even need backrefs,
Yep, that might be worth adding, even if the rest stays the same.
I'll certainly look over what you posted when I get a chance (which won't
be until next week some time), but for now I'm inclined to say the matter
is closed, for this release at least. We've got a working implementation
which it's already on its third iteration (at least) and I think it's
Good Enough for now.
But I'm sure there are plenty of other vmethods that could do with more
thorough testing and/or documenting. We've got a new t/vmethods/ directory
specially for vmethod tests and the existing tests are in there to cut-n-paste
from, or expand upon.
Cheers
A
|
