So, I'm doing a code review with Thing 1 yesterday, that is, I'm reviewing Thing 1's code and not vice versa, and I came across a few issues. Now, in case you don't know what a code review is, I'll tell you. It's when you and one of your peers go over the stuff you just worked on and hopefully catch any potential bugs or violation of best practices or just plain stupidity. I managed to catch all three.
Example 1: Potential Bug
A CAPTCHA was implemented for the first time in one of our projects. Each time the page loads a new image is generated. Thing 1 was generating one image, saving it to disk, and overwriting it every time. Now, maybe I am just being hysterical, but what would happen if say, I don't know, two people hit the page at the same time? Three? Fifty? I told Thing 1 a better option would be to stream the image to the browser. I could tell by Thing 1's blank stare that Thing 1 had no idea what I was talking about.
Example 2: Violation of Best Practices
A button is clicked and a datagrid is loaded. Think of a datagrid as a spreadsheet. Tabular data displayed on a web page. Simple enough. Ah, but open the hood. Thing 1 retrieves the data into a dataset, then checks whether there are more than 0 rows. If there are 1 or more rows, a bunch of functions are executed. In one of those functions, the EXACT same data is retrieved into ANOTHER dataset. I said, why are you retrieving your data twice? Blank stare (I am getting really sick of the blank stare). Thing 1 had no answer. At minumum, I said, you should pass the dataset into the function, but the better solution would be to create a CLASS...
Example 3: Plain Stupidity
"How do I do that? Create a class?"
If you heard a loud noise in Shockoe Bottom yesterday, that was my head exploding.
No comments:
Post a Comment