This page shows the source for this entry, with WebCore formatting language tags and attributes highlighted.

Title

Cleaning up Old Code

Description

Once you've been coding for a while, you'll probably have quite a pile of code that you've written and are regularly using. It's possible that you've got some older code in use that just works and on which you rely every day. At some point, though, you realize that you have to get back in there and fix a few things. That happened recently with the upgrade of the earthli WebCore and attendant applications from PHP4 to PHP5 (which is ongoing). The earthli codebase was born in 1999 and was originally designed to run on PHP3. It has been quite aggresively upgraded and rewritten since then and is thus in pretty decent condition, from the design and stability side of things. The code formatting, however, was old-style and broke a few cardinal rules I'd picked up since 1999. There were two problems in particular that I wanted to address: <ol> Single line statements within conditionals or loops generally lacked braces, which is: <ol>potentially dangerous difficult to read confusing for the Zend debugger</ol> Many final return statements in functions or methods were nested within a useless <c>else</c> statement, e.g.: <code>if ($something_is_true) return true; else return false;</code> </ol> Instead of just living with it, however, I did some global search/replace with regular expressions kung fu to get the code back up to snuff. I used the PCRE support in Zend Studio build 20090119, which I assume just uses the standard Eclipse search/replace support. All operations were applied solution-wide with relatively little trouble. First, I searched for <c>if</c>-statements whose contents did not start with an opening curly bracket: <code> <b>Search:</b> ([ ]+)if \(([^\n]+)\)\n[ ]+([^ {][^\n]+) <b>Replace:</b> \1if (\2)\n\1{\n\1 \3\n\1} </code> From there, it's relatively easy to find/replace all single-line <c>else</c>-statements, by searching for the following: <code> <b>Search:</b> ([ ]+)else\n[ ]+([^ {][^\n]+) <b>Replace:</b> \1else\n\1{\n\1 \2\n\1} </code> Then, I did the more esoteric <c>elseif</c>: <code> <b>Search:</b> ([ ]+)elseif \(([^\n]+)\)\n[ ]+([^ {][^\n]+) <b>Replace:</b> \1elseif (\2)\n\1{\n\1 \3\n\1} </code> And finally, I replaced all loop constructs <code> <b>Search:</b> ([ ]+)foreach \(([^\n]+)\)\n[ ]+([^ {][^\n]+) <b>Replace:</b> \1foreach (\2)\n\1{\n\1 \3\n\1} </code> <code> <b>Search:</b> ([ ]+)for \(([^\n]+)\)\n[ ]+([^ {][^\n]+) <b>Replace:</b> \1for (\2)\n\1{\n\1 \3\n\1} </code> Once I'd normalized all of the <c>else</c>-statements, I could clean up <c>else</c>-statements that included only a <c>return</c>-statement. <code> <b>Search:</b> ([ ]+)else\n[ ]+\{\n[ ]+return ([^\n]+)\n[ ]+\}\n <b>Replace:</b> \n\n\1return \2\n </code> There are a lot more interesting things you can to globally alter your code if you're willing to put some time into building your regular expressions. Legibility is better, debugging works better and there are far fewer warning reported by the compiler.