|go ahead... be a heretic|
I am a senior DBA (Oracle) for a large medium sized company (large medium = 1 billion a year in sales). My education, however, is in Systems Analysis, and as such, I work pretty closely with our application programmers. This week, I was given some perl code to review. The code is for a CGI script which helps people locate the nearest dealer based on ZIP code or city/state. The complaint was that the code was running slower than expected, and I was asked to review the code to see if I could find any problems.
I could not believe what I found in the script. The logic is very hard to follow... there are no subroutines... files are closed without ever being opened... file opens are never checked for success... variable names mean very little... and then we get to the big stuff. *Smiles*
The first two lines are:
however, all HTML is hard coded:
(Yep, that is actual code...)
The next thing I found were horrid comments... well, let me share my favorite three.
The next thing I found were "cool" tricks. Please note, the variable $cnts is never defined.
I'll have to remember that one... just like the one that uses tr to find the length of a string.
Here is how you format a phone number:
Here is how you get the last five characters of a string:
I made a comment about the code to my manager, and have now been called into a meeting with my manager, and the programmer's manager where I am expected to provide feedback on the script and what I think the problems are. If I were the programmer's supervisor, I would have no problem providing him direction (and rejecting the entire script.) However, how can I politely provide factual feedback without "attacking" the style of programming?