[Done]: [Suggestion for code review] get rid of the slow ereg(i) and ereg_replace

[ripat]

Hi,

As I mentioned in another post the ereg(i) functions are notoriously slow and greedy. The can easily be replaced by the PCRE preg_* functions or regular string functions like strpos or str_replace.

This morning, I went trough all files and found a number of occurrences of ereg functions. Here follows my suggestions for replacement. They were all tested for speed. Where my comments says Speed factor +8, it means that my suggestion for replacement is 8 times faster. All speed test were done on loops of 200 iterations.

http://www.lumadis.be/jeanluc/vault/ereg.txt

I still need to go through the following files:
/include/functions.inc.php
/include/picmgmt.inc.php
/include/imageObjectIM.class.php
/lang/dutch.php
/profile.php
/ratepic.php
/register.php
/report_file.php
/upload.php

I'll continue if this suggestion is found useful.

A+
JL

[Joachim Müller]

Yes, I indeed find your suggestions very usefull; I'm looking forward to your suggestions, especially for include/functions.inc.php (as this file contains some crude replacement constructs that have been a thorn in coppermine's side for long.) However, could I suggest adding comments to the non-trivial regular expressions to enable devs and power-users who are not so familiar with regex syntax to come up with possible future edits easily? Having efficient code is great, but for a community effort like coppermine, it's mandatory to enable others to understand the code as well. I consider it one of the defficiencies of coppermine that there is little developer documentation available.

Once again: thank you very much for your suggestions.

Joachim

P.S. As you certainly know your way around, I'd like to encourage you to look at the devel code instead of looking at the code of the stable branch - the devel code is more advanced and it's the code that will actually get improved; for the current stable (cpg1.4.x), only bugfixes will be applied, but no code improvements or additional features. Please refer to the preliminary subversion documentation for cpg1.5.x (under construction). Additional stuff: http://sourceforge.net/svn/?group_id=89658
Folder to check out: https://coppermine.svn.sourceforge.net/svnroot/coppermine/trunk/cpg1.5.x/ (SVN client), http://coppermine.svn.sourceforge.net/viewvc/coppermine/trunk/cpg1.5.x/ (web-SVN)

[ripat]

hi Johachim,

I have never worked on a collaborative project but I am ready to give it a try. However, I will need some coaching on how to use subversion. The tigris.org doc's and faq are more svn server oriented.

I have downloaded the svn client for linux and I am presently working on the regex functions used in v1.5.

Should I make the changes directly on my local v1.5 files - with the necessary comments. How to upload them back to your repository without making any damage?

A+
JL

[Abbas Ali]

You can post your code changes here or give us the patch and we will do the needful. You cannot directly upload/commit in the repository.

[ripat]

OK. There are quite a number of changes. That will be a long post.

Is it OK with this link?
http://www.lumadis.be/jeanluc/vault/cpg15x_txt.php

I will review all the functions tomorrow.

A+
JL

[Joachim Müller]

OK. There are quite a number of changes. That will be a long post.

Doesn't matter - as long as there is good content, we don't mind long postings. And yours is good and valid content

Is it OK with this link?

That's OK just as well. Whatever option you prefer.

Joachim

[ripat]

Well I went through all 1.5.x files that contained ereg() functions and made the changes accordingly. I skipped all files from directory include/.svn/text-base/ that seem to be duplicated from main include/

All changes are here:
text mode: http://www.lumadis.be/jeanluc/vault/cpg15x_txt.php
or
colorized: http://www.lumadis.be/jeanluc/vault/cpg15x_col.php

I still have to check a couple of things. Should be done tomorrow. Also, I have no test script to test the impact of the changes on the application as a whole. It should be no problem as it is quite straight forward to replace ereg by preg_match. Optimization of the pattern for speed is another story. Took me some time...

Of course when I quote a speed gain of, say 10 times faster, it is for the block of code that contains the ereg() function - not for the whole script. But as we say over here "les petits ruisseaux font les grandes rivières" (small streams make the large rivers).

A+
JL

[Nibbler]

Committed to 1.5.