In my small sized department, there's not so much code review as in code testing. Managers and team leaders and users just want the code to work.

Well, my company had introduced offshore developers into the mix. Anyway, I've been tasked to review the code from my China reports. There's ASP.NET web forms, written in VB.NET, plus a sprinkle of Crystal Reports.

My question: any suggestions on how to quickly and easily review code? Accuracy is a must of course. But how about code quality? Do you compromise a bit so you don't keep sending emails back and forth? Any short simple standard procedures you follow?

Do you expect the other coder(s) to write code on par with your standards before deeming it worthy (code-review-wise)?

When I say code quality, I don't mean inefficient code (although there's that...). I mean gems like

and descriptive names such as "Dropdownlist1" for, oh, a dropdownlist (I never would've guessed).

Now let me go think about how to deal with all the SimSun fonts lying sporadically throughout the .aspx files... I've half a mind to grab everything and do it myself. Managed to stop the Not Invented Here syndrome...

Most measures of true quality are, by their nature, subjective. If you are well versed in something, the flaws will be immediately obvious to you. But to someone who doesn't code, or is simply bad at it, the flaws will be invisible. Unless you have some way to quanitify crappy code, some concrete element that makes it bad or wrong, you really can't argue the point.

I guess what I'm trying to say is, a) you're probably out of luck, and the problems you see probably won't be aparent to someone without your level of expertise.

I know that doen't help. It's an honest opinion, though. Don't sweat the small stuff.

Besides "efficiency", I would look for code that is "appropriately maintainable." What does that mean? Well, if you have a static class that you realistically never expect to be extended, then maintainability is not as much of a concern. However, if you do expect to reuse and extend the code, then it had better be easily maintainable and extensible. This means that it should make good use of modern OO concepts and patterns. For example, if you notice that simple future additions would each require many small changes in many places then someone needs to go back to the drawing board. I'd reject it and give them some feedback on what I wanted to see. Another example of something I'd reject is JSP apps that violate the MVC pattern. Trust me - if you've ever had to work on LARGE J2EE applications written before the JSP 2.0 standard (and Struts) then you know that it is painful and takes people a lot longer to get up to speed on how the app works and therefore how to troubleshoot it.

The customer may not care how the app is implemented. They just want it yesterday and they want it to work. But if you let that mentality creep into your shop (or you let crappy code in from your reports) then the result will likely be crappy, unmaintainable code that will make new bugs more likely and troubleshooting them more difficult. This may cause you to miss release dates. This may also cause employees to want to work elsewhere. It's just generally a bad idea.

This post has been edited by Programmist: 20 Nov, 2007 - 11:32 AM

It's the time-cost-quality triangle, the constant struggle with management. The developers want their code to work, without bugs, flaws, or errors. However, management wanted it two weeks ago, & just told the developers about it today. They don't care how, just get it done.

Unfortunately, because it is a triangle, you'll only ever meet two of the criteria, & never all three.

I thought so... *sigh*

I'm going to strive for the balance in the time-cost-quality triangle, with a bias towards maintainability.

The offshore developers are fresh grads for the most part. They've barely heard of VS2003 let alone use it. Yes, I know VS2008 launched recently. My company's on the "conservative" side...

Anyway, the practice so far's been to use existing code as an example or template, with which they'll modify to the actual requirements. Granted the original code's not quite up to standard, but I'm just the takeover guy...

I'm supposed to be training them. Perhaps a more academic approach would appeal to them better... like hints and stuff. ... Strong hints. I still need completed work before deadlines

So, this is what I'll do:
1) Get accurate working code first
2) At the same time, guide them on basic UI and code principles (they seem oblivious to atrocious aesthetics both in UI and code).
3) Get them to correct obviously incorrect coding practises

The rationale is that at any one time, I have working (or close to working) code for release. Then it's a matter of mustering willpower to write that one more email to tell them to correct code to an acceptable standard (even though it's already working).

The other obstacle will be language. English isn't their native tongue, so I'll have to be careful about ambiguous wordings. They follow instructions to the letter, sometimes literally. Frankly speaking, it usually takes less time to write the code myself than to write enough detailed instructions/requirements for them.

Thanks everyone for your feedback!

I find it interesting that, of all of the comments that the three of us gave, you keyed on that one. It's almost as if that's what you wanted/expected to hear. I can tell you, without a doubt, that if you think you are "out of luck", then you definitely are.

Haha... you know what, I think you're right. I'm going to go brain wash myself in positive thinking for a few minutes. Excuse me...

* go do deep breathing exercise. drink glass of cold water *

Well, it just happens that I read baavgai's comments first. And it struck me as particularly funny. And yes, I've had similar thoughts to all three of your comments before I posted the original question. Just wanted to know if maybe people in the West (or other countries) do things a little differently that I can learn from.

How does one go about learning/knowing what standardized coding is? I just recently graduated, have had no work experience to speak of.

I must say that whenever i do anything in VB .net, its all variable names i use a lot. They are common in my code and i can easily understand what they do, but when someone else reads my code they are also somewhat easy to understand. Some of the people i work with though have such descriptive names that you cannot understand them. They use like intNSalesTotal. Thats a lot harder to undertsand then North.