Friday, May 1, 2009

Get Rid of Dead Code

How many Siebel developers believe that the best way to remove lines of code from an existing script is to put some comment characters in front of the un-needed code, preventing it from executing. Many developers will copy an existing line of code to change an operation or add a condition, "commenting" the original version instead of deleting it. -- Added 5/3

Good coding practice tells us to diligently and carefully comment and annotate changes to scripts, right? Lets take a step back for a moment and think it through. I'm not certain that the commented lines of script are very helpful, at least in most cases.

First of all, commented script rarely helps to trace the historical changes made in a script. If the commented changes are dated, as they often are, you might find the date helpful. But script comments don't lend themselves to historical analysis nearly as well as versioning. Comparing a previous version of a script with the current version, in a source control repository for example, is much easier than trying to decipher the versions from comments in the script itself. Especially considering that the comments from several changes are often mixed in together.

Secondly, commented script doesn't help much in understanding how the current version of the script works. Lets face it: when you examine a script, you are not looking at the code that is commented. You skip the comments and focus on the code that is still active. If the developer happened to leave some actual words among all the dead code, you might stop to read it, but you certainly won't spend much analysis to find out how the script used to work.

Thirdly, and perhaps most importantly, dead code can cause significant overhead on the Siebel server. Every development team that habitually comments unused blocks of script will soon find that an entire event handler has been commented. In fact, it is common to find Siebel repositories with multiple event handlers entirely commented out. Also common are scripted event handlers where no script is present at all. Event handlers are marked as scripted even if the script contains no executable code except the application-provided stub:

function WebApplet_PreCanInvokeMethod (MethodName,
&CanInvoke)
{
return (ContinueOperation);
}

These event handlers cause a measurable increase in CPU load, especially when the event is frequently encountered. The empty scripts also become a maintenance issue, increasing the time required to compile an srf.

So, get rid of all that dead code. If a scripted event handler is empty, completely delete the script record in the Tools list applet. If you are removing individual lines of script, make sure you fully document your changes in design documents, and use a source control system to keep previous versions of the script. But keep the script itself clean. A clean repository will be easier to maintain and provide less overhead on your Siebel server.