Jump to content
  • Advertisement

Archived

This topic is now archived and is closed to further replies.

liquiddark

Code Audits/Reviews

This topic is 5143 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

Has anyone had any experience with code-level peer review? If so, any answers to the questions below would be greatly appreciated: What was involved in pulling a review together? What were the triggers for a review? What was the net effect in code quality terms? Was there a perceived increase in concientiousness by the developers on the team? What problems arose as a consequence? Thanks, ld

Share this post


Link to post
Share on other sites
Advertisement
Guest Anonymous Poster
The place I worked at 10 years ago instituted code reviews. Nobody has since. I think I was the first person assigned to review someone else''s code. It ended up being a waste of time. Nobody had done them before. Nobody cared about it. If anything, people didn''t like other people looking around their code. The attitude was that as long as it compiled and ran, who gave a crap about anything else. Too bad since code reviews are supposed to be very effective if done properly.

Share this post


Link to post
Share on other sites
I was a device driver developer for Symbios Logic and they did extensive code reviews. My company also has very extensive code reviews as well. Here are my answers to your questions

What was involved in pulling a review together?
1. Make sure the code is to the coding standards. It is a good idea to have company coding standards. It makes code easier to read by everyone, and everyone doesn''t have to ramp up.

2. Checked into source control, locked down and frozen

3. Make sure that the developer has unit tested the code, and has a ITS (integrated test specification) that tells all the tests that where performed on the code in a unit test (unit test means only the developer tested it on a couple machines)

4. Make sure all dead code (commented out) is removed. And the code is commented.

5. Then we run the code through something like 4print that will print it out and number the lines, and title the top of the pages with the filename, and page number (easier to track for multiple file projects)

6. Then we take the printed copy to Kinkos and have a spiral bound version of that locked down code printed for each person that will be reviewing the code. (For us this is usually 3-6 people)

7. Then we hand the printed copies out, and give everyone a week to look through / red ink their copies.

8. Schedule a meeting, or series and review the code outloud.

9. KEEP EMOTIONAL LEVELS IN CHECK! I have seen these things get out of hand quickly. Especially is someone finds a major screw up.

What were the triggers for a review?
There are no triggers. We ALWAYS review our code before shipping a product. Hence we have NEVER had to release a patch, or fix

What was the net effect in code quality terms?
Well, we hold developers to a very very high standard of code development. The review is good because you get other people to look at the code. Quite often they find things that the developer didn''t or couldn''t see because they were too close to the problem. Also a lot of times someone may suggest a more efficient method of doing something. Thus, improving the code.

Was there a perceived increase in concientiousness by the developers on the team?
You bet! When people know that there work will be viewed and picked a part by others, they tend to try a little harder. Those that don''t are slackers and usually after a review for them, everyone else suggests letting them go, and we do

What problems arose as a consequence?
The only problems with reviews is the emotional levels. Someone will think something should have been done differently and sometimes people argue about the best ways to solve problems. The key is that when your code is under review. Don''t take anything as a personal attack. It is a review of your code, not of your person. Other than that I have seen nothing but improvements come out of them if they are handled in the manner perscribed above

Hope this helps you.



Sincerely,
Randy Trulson
www.GamePotato.com
www.NeuronGames.com

Share this post


Link to post
Share on other sites
Quote:
Original post by liquiddark

Has anyone had any experience with code-level peer review?


Relativial new.

> What was involved in pulling a review together?
We're required to do them, and we just schedule a conference room and email-invite the programmers on the team as well as a couple of other people; sometimes some bodies from other projects. Print out the code about a week ahead of time to give everyone a chance to look it over prior to the meeting.

> What were the triggers for a review?
Whenever new code is written.

> What was the net effect in code quality terms?
Hard to tell as of yet.

> Was there a perceived increase in concientiousness by the developers on the team?
It is certainly is driving me to set a (worthy) example for the younger guys to follow. My unit-test are more comprehensive, and I comment more since I know other people will be looking at the code. I double-checked all my entrance and exit assertations, I double checked every routine for thread-safety and exception -safety, I ensured I used automatic management for every resource I could, etc... One rule I always break is putting too much code in headers, so it's cut down on that too.

> What problems arose as a consequence?
I had a sit down with management and asked them why they hired half a dozen young Java programmers for a C++ project and what the plan was to give them the training and tools they need to be productive (there isn't one... sigh).

Share this post


Link to post
Share on other sites
Wow cool "Randy of Neuron", I wish I was working there.

I wouldn't mind others seeing my wonderful self-documenting code at all. Especially my nice tidy Delphi code.

:-)

Share this post


Link to post
Share on other sites
They used code reviews extensively at my last work place

>What was involved in pulling a review together?
Make sure you're code was in check with the coding standard. Check that it compiled in Debug and Release mode in MS Visual Studio and outside of Visual Studio. I screwed up nicely once because I didn't do that. Then making network accesible folders where one folder contained the code before changes we're made and one folder contained the code after changes we're made. The reviewer reviewed the code within those folders on his/her own time using Araxis Merge or someother diff tool and recorded a bunch of suggestions. The writer would then review the reviewers suggestions with the reviewer for clarfication of any of suggestions made. The writer would then go back and implement any necessary changes and if necessary the process would repeat. That would generally only happen for serious issues such as a crash.

Pretty extensive but it was necessary considering this was for medical imaging software.

>What were the triggers for a review?
Whenever new code was written.

>What was the net effect in code quality terms?
Can't really say as the reviews we're there years before I joined but it defeinitely helped improve my coding abilities. As was mentioned already you're more careful in you're coding practices when you know someone else will be looking at it.


Share this post


Link to post
Share on other sites

  • Advertisement
×

Important Information

By using GameDev.net, you agree to our community Guidelines, Terms of Use, and Privacy Policy.

We are the game development community.

Whether you are an indie, hobbyist, AAA developer, or just trying to learn, GameDev.net is the place for you to learn, share, and connect with the games industry. Learn more About Us or sign up!

Sign me up!