Code reviews
-
Hi, I would suggest using a static analyzer tool in case code reviews is not acceptable. Regards, Kiran
There is no better solution to this everlasting problem other than automating the process.
-
I work as a developer in a small software company. The software we produce has a high deffect rate. I want to talk my boss into doing code reviews but the problems that I´m facing are that I don´t find my boss and the other senior developers mature enough to do a real code review instead of a "mine is biggers than yours" code review (they use to talk thrash about other programmer´s code, just because it´s not programmed their way) and that the company promotes individualism insted of team playing (and a code review could be used not to find bugs but to diminish someone else code so they can stand out) So, I do want to get a better software but I´m afraid I could start a monster... Any idea?
I had the same issue with a company where I worked. There were 2 major ego people (the 2 leads on the project). They tried so hard to be the "best". Their egos couldn't fit into the building. They always had issues with people's coding. On EACH project they went, the complained about people (developers). Now my questions is how do you have these 2 people review anything?
-
I work as a developer in a small software company. The software we produce has a high deffect rate. I want to talk my boss into doing code reviews but the problems that I´m facing are that I don´t find my boss and the other senior developers mature enough to do a real code review instead of a "mine is biggers than yours" code review (they use to talk thrash about other programmer´s code, just because it´s not programmed their way) and that the company promotes individualism insted of team playing (and a code review could be used not to find bugs but to diminish someone else code so they can stand out) So, I do want to get a better software but I´m afraid I could start a monster... Any idea?
#1: Don't try to run code reviews in that atmosphere. #2: start looking for a new job. Even if the rest is OK for now, this is no a good place to settle down. #3: Figure out why there are so many bugs. #4: Try unit tests for yourself, see if it works for you, and if it imresses others.
-
Indeed - and I'm a big fan of his work, but code reviews are rarely done by people who can put their egos to one side. And this is a huge problem with code reviews - everybody has to sign up to it, and they have to embrace it. Code reviews have to be equal - everybody has to be treated equally. This is all external process. So, what can you do to improve your code - well, you can actively look to test your code. Exercise it and try to break it. With the advent of tools like nunit, there's no reason not to test. Plus, Code Complete was originally written before automated testing tools such as NUnit were widely available and used. I rather suspect that Steve McConnell would embrace them as well.
Deja View - the feeling that you've seen this post before.
Pete O'Hanlon wrote:
but ...code reviews are rarely done by people who can put their egos to one side.
How true! :( To original poster: Finding a new job would be much better. I've worked in a surrounding where those who were low on the food scale were always blamed for every fault on project.
-
I work as a developer in a small software company. The software we produce has a high deffect rate. I want to talk my boss into doing code reviews but the problems that I´m facing are that I don´t find my boss and the other senior developers mature enough to do a real code review instead of a "mine is biggers than yours" code review (they use to talk thrash about other programmer´s code, just because it´s not programmed their way) and that the company promotes individualism insted of team playing (and a code review could be used not to find bugs but to diminish someone else code so they can stand out) So, I do want to get a better software but I´m afraid I could start a monster... Any idea?
Good question! I would agree with most folks above. When I started in my present position code-reviews were never heard of. But the atmosphere was very friendly with nothing of that ego-stuff. Our coding was happy-go-lucky, with very inconsistent quality. A new guy introduced the idea of reviews, it was not met without resistance, but he didn't give in, and gradually his ideas were embraced. He is one of best programmers I have met, and I think that if he hadn't been respected, his ideas might have fallen to the ground. I would like to ephasize/add/clarify that for code reviews to be helpful everybody has to embrace the idea. Also you will probably need some document with coding guidlines. Those guidelines can include simple stuff such as naming, formatting and commenting conventions. But also they should contain structural guidelines: use of frameworks (STL, Boost etc), avoidance of code duplication. Also organization of common classes etc. (TDD etc is great but it seems very far off in your organization.) All this should be in a document in order to aviod silly discussions like: - I don't think it is necessary to have all constants in upper-case - Well, I disagree So, maybe, your first fight should be for such a document, not the reviews directly. And, find an ally before dropping the bomb! Good Luck!
_____________________________________ Action without thought is not action Action without emotion is not life
-
Just asking a trusted mate to look over your code or bouncing an algorithm off of them. Sometimes it's just scribbling on a white board as you work something out verbally.
Anyone who thinks he has a better idea of what's good for people than people do is a swine. - P.J. O'Rourke
To the extreme end of this, you might even look at pair programming, even if it is only one day a week. If you don't know, pair programming is a type of informal code review where two developers are sat in front of the same computer, one "drives" while the other simply looks over their shoulder and watches for other ways of doing things (not necessarily better, just other ways). The two can switch places whenever they feel it necessary. Be warned that many people are opposed to pair programming, most will say they don't like someone looking over their shoulder or they don't want to lose personal space. There also is a very common misconception that the pair will only get 1/2 the amount of work done. This is not true for experienced teams, as the chance of unknown areas declines with having two programmers, and the code should be more solid, which saves maintenance time. One other terrific way of improving development processes is to set up a continuous build environment such as CruiseControl.NET. Simply set up the server (a 4-6 hour task for someone new to CCNET) and then set up a notification service, be it email, a notification icon (CCTray is the one for CruiseControl.NET), or a pair of lava lamps (I have seen this, it's very effective). The hardest part will then be getting the developers to do constant checkins, as the build is triggered by a checkin. Then, whenever new code is added, a build is automatically started and any unit tests you have setup will be run. If the build or the tests fail, notifications will be sent out, the notification icon will turn red and popup a message, and the team is now responsible for fixing the build. CCNET will list what build failed and why, and what files were changed and by who. Since you know what files to look at, and you know that fairly few lines of code could have changed (b/c of the frequent checkins), the problem should be able to be fixed quickly. Anyways, I hope the above gives you some more ideas on how to proceed. The true test of whether or not you should stay or leave your current job is in management reaction. If management is not willing to invest their time into fixing their procedures, then I would start to wonder if they are a company that's good for you to work for. Thanks, Seth Rowe [MVP] http://sethrowe.blogspot.com/
-
To the extreme end of this, you might even look at pair programming, even if it is only one day a week. If you don't know, pair programming is a type of informal code review where two developers are sat in front of the same computer, one "drives" while the other simply looks over their shoulder and watches for other ways of doing things (not necessarily better, just other ways). The two can switch places whenever they feel it necessary. Be warned that many people are opposed to pair programming, most will say they don't like someone looking over their shoulder or they don't want to lose personal space. There also is a very common misconception that the pair will only get 1/2 the amount of work done. This is not true for experienced teams, as the chance of unknown areas declines with having two programmers, and the code should be more solid, which saves maintenance time. One other terrific way of improving development processes is to set up a continuous build environment such as CruiseControl.NET. Simply set up the server (a 4-6 hour task for someone new to CCNET) and then set up a notification service, be it email, a notification icon (CCTray is the one for CruiseControl.NET), or a pair of lava lamps (I have seen this, it's very effective). The hardest part will then be getting the developers to do constant checkins, as the build is triggered by a checkin. Then, whenever new code is added, a build is automatically started and any unit tests you have setup will be run. If the build or the tests fail, notifications will be sent out, the notification icon will turn red and popup a message, and the team is now responsible for fixing the build. CCNET will list what build failed and why, and what files were changed and by who. Since you know what files to look at, and you know that fairly few lines of code could have changed (b/c of the frequent checkins), the problem should be able to be fixed quickly. Anyways, I hope the above gives you some more ideas on how to proceed. The true test of whether or not you should stay or leave your current job is in management reaction. If management is not willing to invest their time into fixing their procedures, then I would start to wonder if they are a company that's good for you to work for. Thanks, Seth Rowe [MVP] http://sethrowe.blogspot.com/
I totally agree with Seth Rowe. I have set up CruiseControl .net. You do not have to broach the subject of code review initially. Let Cruisecontrol compile the code every time and if possible start putting in the NUnit Tests as much as possible. The quality will start improving immediately.. and those that see their code fail multiple times will have to start thinking of better ways.. code review may eventually follow and it may come from higher up without you really saying anything... CCNET + NUnit really helps ! -Prashant
-Prashant
-
To the extreme end of this, you might even look at pair programming, even if it is only one day a week. If you don't know, pair programming is a type of informal code review where two developers are sat in front of the same computer, one "drives" while the other simply looks over their shoulder and watches for other ways of doing things (not necessarily better, just other ways). The two can switch places whenever they feel it necessary. Be warned that many people are opposed to pair programming, most will say they don't like someone looking over their shoulder or they don't want to lose personal space. There also is a very common misconception that the pair will only get 1/2 the amount of work done. This is not true for experienced teams, as the chance of unknown areas declines with having two programmers, and the code should be more solid, which saves maintenance time. One other terrific way of improving development processes is to set up a continuous build environment such as CruiseControl.NET. Simply set up the server (a 4-6 hour task for someone new to CCNET) and then set up a notification service, be it email, a notification icon (CCTray is the one for CruiseControl.NET), or a pair of lava lamps (I have seen this, it's very effective). The hardest part will then be getting the developers to do constant checkins, as the build is triggered by a checkin. Then, whenever new code is added, a build is automatically started and any unit tests you have setup will be run. If the build or the tests fail, notifications will be sent out, the notification icon will turn red and popup a message, and the team is now responsible for fixing the build. CCNET will list what build failed and why, and what files were changed and by who. Since you know what files to look at, and you know that fairly few lines of code could have changed (b/c of the frequent checkins), the problem should be able to be fixed quickly. Anyways, I hope the above gives you some more ideas on how to proceed. The true test of whether or not you should stay or leave your current job is in management reaction. If management is not willing to invest their time into fixing their procedures, then I would start to wonder if they are a company that's good for you to work for. Thanks, Seth Rowe [MVP] http://sethrowe.blogspot.com/
I think I'll expand a bit on the lava lamp idea, as it really is a good idea. I originally heard about it from Andy Hunt, one of the Agile founders, during one of his presentations. I'm sure some of the exact details are missing, but it basically goes like this: A team set up a pair of green and red lava lamps in the team room with full visibility to management and had their power sources connected to the build server. When a build would succeed, the green lava lamp would be powered, signaling that all was well. If however the build were to fail, the the green lamp would turn off and the power would be switched to the red lamp. The interesting thing is that lava lamps take a bit to warm up and start flowing, so there would be a time span after a broken build when neither lamps were lit. These lamps were in the team room and had full visibility to management. Depending on the status of the lamps, management could tell the following: 1) Green Lamp - All is well on the project. No involvement necessary. 2) No Lamps - A build error has occurred, but the team is handling it. No involvement necessary. 3) Red Lamp - Significant problem. Might be time to check with the team and see if help is needed. Thanks, Seth Rowe [MVP] http://sethrowe.blogspot.com/
-
I work as a developer in a small software company. The software we produce has a high deffect rate. I want to talk my boss into doing code reviews but the problems that I´m facing are that I don´t find my boss and the other senior developers mature enough to do a real code review instead of a "mine is biggers than yours" code review (they use to talk thrash about other programmer´s code, just because it´s not programmed their way) and that the company promotes individualism insted of team playing (and a code review could be used not to find bugs but to diminish someone else code so they can stand out) So, I do want to get a better software but I´m afraid I could start a monster... Any idea?
-
Code reviews aren't a magic bullet for solving issues of code quality. If your software has a high defect rate there are deeper issues you need to address. You need to take a look at the root causes of your defects, and the most likely cause is a lack of rigorous testing. A good start would be to introduce automated unit testing into your regime, and possibly look at things like Test Driven Development. Unit Testing is a discipline that needs to be worked at but, done properly, it is worth doing and does help.
Deja View - the feeling that you've seen this post before.
Pete O'Hanlon wrote:
Code reviews aren't a magic bullet for solving issues of code quality. If your software has a high defect rate there are deeper issues you need to address
Hey, you have to start somewhere and there is eveidence that code reviews help. Unit testing does help but "Unit Testing isn't a magic bullet for solving issues of code quality."
-
I think I'll expand a bit on the lava lamp idea, as it really is a good idea. I originally heard about it from Andy Hunt, one of the Agile founders, during one of his presentations. I'm sure some of the exact details are missing, but it basically goes like this: A team set up a pair of green and red lava lamps in the team room with full visibility to management and had their power sources connected to the build server. When a build would succeed, the green lava lamp would be powered, signaling that all was well. If however the build were to fail, the the green lamp would turn off and the power would be switched to the red lamp. The interesting thing is that lava lamps take a bit to warm up and start flowing, so there would be a time span after a broken build when neither lamps were lit. These lamps were in the team room and had full visibility to management. Depending on the status of the lamps, management could tell the following: 1) Green Lamp - All is well on the project. No involvement necessary. 2) No Lamps - A build error has occurred, but the team is handling it. No involvement necessary. 3) Red Lamp - Significant problem. Might be time to check with the team and see if help is needed. Thanks, Seth Rowe [MVP] http://sethrowe.blogspot.com/
If you do not want to get into lava lamp just yet, Just set up the CruiseControl.net server. It gives you a fantastic and Free webdashboard. Disable the FORCE compile on the web-dashboard and send the link(usually http://yourcomputername/ccnet) to your management. It will do the trick !
-Prashant
-
I work as a developer in a small software company. The software we produce has a high deffect rate. I want to talk my boss into doing code reviews but the problems that I´m facing are that I don´t find my boss and the other senior developers mature enough to do a real code review instead of a "mine is biggers than yours" code review (they use to talk thrash about other programmer´s code, just because it´s not programmed their way) and that the company promotes individualism insted of team playing (and a code review could be used not to find bugs but to diminish someone else code so they can stand out) So, I do want to get a better software but I´m afraid I could start a monster... Any idea?
Gabriel.P.G wrote:
I´m facing are that I don´t find my boss and the other senior developers mature enough to do a real code review instead of a "mine is biggers than yours" code review (they use to talk thrash about other programmer´s code, just because it´s not programmed their way)
Sounds vaguely familar. My company uses code reviews to try to force people to code the way the most senior develops "tells" people to code. (He doesn't even follow the rules he tells others to follow)
-
I work as a developer in a small software company. The software we produce has a high deffect rate. I want to talk my boss into doing code reviews but the problems that I´m facing are that I don´t find my boss and the other senior developers mature enough to do a real code review instead of a "mine is biggers than yours" code review (they use to talk thrash about other programmer´s code, just because it´s not programmed their way) and that the company promotes individualism insted of team playing (and a code review could be used not to find bugs but to diminish someone else code so they can stand out) So, I do want to get a better software but I´m afraid I could start a monster... Any idea?
I actually think that code reviews are pretty darn close to a magic bullet! Admittedly, this is an extreme statement, but my own experience as well as well-documented research seems to agree. My experience is that a simple "talk through" of the code, line by line, routes out MANY (if not most) bugs. It helps if you have some coding standards in place (even "gentleman's agreements" about naming conventions, etc.) to give everyone a common ground for "criticism". I hardily recommend "Code Complete" for guidance. You should insist that your peers comply with basic good practices like commenting code, reasonable variable names, etc. To convince your bosses about this, why not copy or print (yes, on real paper) an article or two with some hard evidence and give it to them without explicitly telling them that you're trying to sell them on reviews. Then, let THEM come up with the idea of mandating code reviews. Help them think it was their idea. If you get push back from your management, then they clearly are not interested in improving your organization and it may be time to move on...
-
Several places I've worked have tried formalized code reviews. It's never worked. Informal code reviews have worked well, but entirely depend on quality engineers.
Anyone who thinks he has a better idea of what's good for people than people do is a swine. - P.J. O'Rourke
Take a look at http://www.review-board.org/[^] Its a great tool, we use it before every check-in.
-
I work as a developer in a small software company. The software we produce has a high deffect rate. I want to talk my boss into doing code reviews but the problems that I´m facing are that I don´t find my boss and the other senior developers mature enough to do a real code review instead of a "mine is biggers than yours" code review (they use to talk thrash about other programmer´s code, just because it´s not programmed their way) and that the company promotes individualism insted of team playing (and a code review could be used not to find bugs but to diminish someone else code so they can stand out) So, I do want to get a better software but I´m afraid I could start a monster... Any idea?
I have promoted software engineering concepts for years in two different companies with absolutely no success. Stephen McConnell, one of the leading software engineering analysts in the United States today has often stated that attempting to bring in such concepts and techniques is often a very difficult and unsuccessful endeavor. Software engineering requires a level of intelligence that is often not found with technical management...
Steve Naidamast Black Falcon Software, Inc. blackfalconsoftware@ix.netcom.com
-
I work as a developer in a small software company. The software we produce has a high deffect rate. I want to talk my boss into doing code reviews but the problems that I´m facing are that I don´t find my boss and the other senior developers mature enough to do a real code review instead of a "mine is biggers than yours" code review (they use to talk thrash about other programmer´s code, just because it´s not programmed their way) and that the company promotes individualism insted of team playing (and a code review could be used not to find bugs but to diminish someone else code so they can stand out) So, I do want to get a better software but I´m afraid I could start a monster... Any idea?
In my 20+ years, I have never seen code reviews do more than correct minor programming errors, and waste time. I know this is going to rile up some folks, but based on the reality of my experience, I have decided that code reviews are a nice concept, but never productive. The longer I code the more I recognize the shiny things that make my work slower... like introducing more and more process and admin, that magically reduces my coding hours...in theory. We have not moved very far in the software world, I think we all agree that the physical construction business is one we envy for their "Blue-Prints" and repeatable, modular techniques. Software can never really get to this level due to many factors... not the least of which is that our construction techniques, and process change far to often and too radically. Not just my opinion. My experience. I know you have different experiences, and thats ok... I still believe somewhere out there someone is doing all these things right. I just have never seen it.
Where there's smoke, there's a Blue Screen of death.
-
Several places I've worked have tried formalized code reviews. It's never worked. Informal code reviews have worked well, but entirely depend on quality engineers.
Anyone who thinks he has a better idea of what's good for people than people do is a swine. - P.J. O'Rourke
I worked for over 8 years at a company that had a formal code review process that was essentially integrated into the development life cycle (they worked on contracts requiring quality standards like CMMI and such). It worked very well there as it was a REQUIRED part of the process where a separate Quality Assurance engineer who understood code well enough to read it but did not actually write it and thus had no ego invested. The developer would put together a review package which was sent to other team members, the team lead, the QA engineer, and (when we could get one) someone outside the team (thus, they had no ego invested in that code either). We had a management-approved coding standard and they also agreed to having code review time (and unit testing) built into the schedule. QA also had the authority (though they never needed to exercise it) to prevent the entire contract from advancing to the next stage because they were required to sign off on it. If the QA mentioned that they were concerned about someone who resisted the process, you can be sure that management would get on the develper's case and they would either adjust their attitude, get reassigned, or start looking for another job. Similary, we would try to enforce constructive reviews and focused on looking for functional (not style) problems or made suggestions about alternatives (XX wrote a utility class which would simplify that, ...). If someone became nasty or non-constructive, they would get a visit from management. This worked because of complete management backing and the fact that developers were not allowed to short change reviews and testing due to the schedule (maybe because the customer originating the contract had to accept it with a formal test procedure approved by separate test engineers and QA as well, and it was also not tied to things like marketing and release dates). I've also been working for another large company that does not do them, but has occasionally expressed interest in it. I think that it will not really get anywhere until management really throws support behind it as it needs to become more of a "cultural" part of the process with time allocated towards the process. QA here is also more involved with testing in multiple environments and trying to find problems, but they never look at code (and I doubt most if any of them would really be able to understand it well enough to even "read" it)
-
I work as a developer in a small software company. The software we produce has a high deffect rate. I want to talk my boss into doing code reviews but the problems that I´m facing are that I don´t find my boss and the other senior developers mature enough to do a real code review instead of a "mine is biggers than yours" code review (they use to talk thrash about other programmer´s code, just because it´s not programmed their way) and that the company promotes individualism insted of team playing (and a code review could be used not to find bugs but to diminish someone else code so they can stand out) So, I do want to get a better software but I´m afraid I could start a monster... Any idea?
Code reviews are not great for debugging code, compilers and testing are much more efficient at that. Where code reviews are very useful is with algorithms, efficiency, and teaching. We think we know a lot when we start out programming, but experience brings a lot to the table. People with more experience on the platform or the field you are programming in can easily catch faulty assumptions that you would never see. Also, new guys can bring in ideas that you are unaware of because you took your last class twenty years ago. I did a lot of code reviews at IBM, CSC, WordPerfect, but haven't done any in a long time. You have to put your ego aside, but I learned a lot from them. Now my code reviews consist of looking at someone else's code to see if I can find a bug that he can't see. I think we (in my current employment situation) are missing some great teaching and learning opportunities.
SS => Qualified in Submarines "We sleep soundly in our beds because rough men stand ready in the night to visit violence on those who would do us harm". Winston Churchill
-
I work as a developer in a small software company. The software we produce has a high deffect rate. I want to talk my boss into doing code reviews but the problems that I´m facing are that I don´t find my boss and the other senior developers mature enough to do a real code review instead of a "mine is biggers than yours" code review (they use to talk thrash about other programmer´s code, just because it´s not programmed their way) and that the company promotes individualism insted of team playing (and a code review could be used not to find bugs but to diminish someone else code so they can stand out) So, I do want to get a better software but I´m afraid I could start a monster... Any idea?
Code reviews are a key facet of any mature development process, as are unit tests, integration testing with published test plans, functional testing, performance engineering, etc. I work in the financial industry where defect management is very important, although perhaps not as important as the airline industry, nuclear power, automobile, etc. where people's lives could be at stake, but close. So process is critical to ensuring quality. What is required is mature leadership, which sounds to be lacking at your organization. There are lots of models to follow, and frankly everyone's model is a bit different even if it goes by the same name. Our particular model takes advantage of lower cost offshore developers with project leads/managers both on site and offshore. We have junior and senior devs offshore, but frankly, the seniors are still pretty fresh, and the juniors are really raw beginners in a lot of cases. Still we seem to make it work because we're loaded with process and everything, from assessments and SWAGs to QA strategy to functional test plans to production code, everything gets reviewed. And nobody takes it personally, from the senior architect (me) to the designers, QA personnel, and lowest programmer. Everyone buys into the philosophy when they join the team or they don't join. And we've never had a problem with this.
-
I work as a developer in a small software company. The software we produce has a high deffect rate. I want to talk my boss into doing code reviews but the problems that I´m facing are that I don´t find my boss and the other senior developers mature enough to do a real code review instead of a "mine is biggers than yours" code review (they use to talk thrash about other programmer´s code, just because it´s not programmed their way) and that the company promotes individualism insted of team playing (and a code review could be used not to find bugs but to diminish someone else code so they can stand out) So, I do want to get a better software but I´m afraid I could start a monster... Any idea?
Everyone has provided great comments but I wanted to throw in a bit of personal experience. There is no magic bullet for fixing your software defect issues, and applying a tool in the wrong way can be more damaging than helpful. You can't take a cookie cutter process and apply it to every organization and expect it to work. There are some basic concepts that do apply that can be very helpful. First things first, if you are trying to change an organization from the bottom up you have to start slow and fix yourself first. I would recommend trying out Test Driven Development where you write your unit tests based on requirements before writing a single line of code. Each time you think you satisfy a requirement, build and run the tests. You will write better code faster and only add features that are required (it helps you focus). Once you can show how it has improved your coding, have a mate (not the same one each time so as to get a lot of exposure) look over your code occasionally for an informal code review and introduce them to the TDD if they are open to listening. If you can start to sell the process to a few folks, you will see productivity start to grow and defects start to drop in the group for those that are using the process. Now you can approach management with what you have done and show the results, they will be more willing to listen because it is no longer academic theory but visible results. When you go to a manager to try to get something to change, bring them a plan to make the change and something to show that the change will be worthwhile, if you just say we need a change but dont provide a plan and evidence it is hard to decide to drop what the manager is working on to figure out what you are trying to do. Remember, change is a lot of work and no one wants more work unless it means less work in the medium/long term. I recommend the following books for ideas on improving processes and code in general: Code Complete 2nd Edition by Steve McConnell[^] Extreme Software Engineerin