Software Engineering Stack Exchange is a question and answer site for professionals, academics, and students working within the systems development life cycle. It only takes a minute to sign up.
Sign up to join this community
I've worked at two companies, who each had a different methodology when it came to code reviews:
In the first company, a code review was conducted by the team leaders and was required after the completion of every module.
However, in the second company, team leaders weren't required to conduct any code reviews, and just checked for functionality and design issues.
So I am confused. Is the code review process really needed? If it is, why? And if it isn't, why not?
I personally think that every piece of code should go through a code review, it doesn't matter if you are junior or senior developer.
Why? For starters your title doesn't state anything about how you develop, and a senior developer could learn something from the junior. At our company we shift around so one of the other members of the team review your code...mostly we are teamed a "junior" and a senior together, so all the stuff that doesn't get said on a daily basis can be caught in a follow up. If the senior doesn't like the junior code he should listen to why the junior did as he did and look at it and see if that's a feasible solution that might be used in the future...it's a matter of getting wiser no matter who you are.
One important thing about code review is not being too nice, if you are being a nice guy you'll just allow more and more messy code to evolve in the system. Just as yesterday I started reworking a complete application that a former employed juniordeveloper wrote, and my god that code could have needed a review before he left.
I don't see why it should be the teamleader only doing reviews but it requires a person that's not affraid of picking a "fight" over a piece of poorly developed code, and it has to be a person that cares about how the code should be. Not all companies hire people that actually care about what they do, and those bad eggs should IMO not be allowed to do code reviews as they are likely just to shrug their shoulders and say "OK" to bad code.
Basically code review is needed for all programmers regardless of experience. It is the Quality Control of software development, and one of the reasons Open Source can be of very high quality.
EDIT: The reason is that a code reviewer today is in exactly the same role as a maintainer later. If the code doesn't make sense to him today, it will not make sense later either, making bugs more expensive to fix. Hence, MAKE it understandable today while the developer still remembers the code. Also the reviewer may see bugs or omissions that the developer missed.
Unfortunately very few wants to do it, but seen from a business point of view it should be mandatory.
I work in a place where code review is now a requirement but was not as little as 3 years ago. It has made a huge improvement in in our code and in the ability of others to maintain the code later. Even senior, very experienced developers make mistakes which can easily and quietly be fixed in code review before QA finds them or worse before the customer finds them. Further at least one person other than the orginal developer is familar with the code.
Often when an organization tries something new to it, as we did with code review, there is much resistance to change. I have seen almost none of that (we were estatic to get a formal QA department too.) with code review. It pretty much takes only one or two reviews to see the value.
I have found new techniques I had not considered both in doing a code review of someone else's work or in having my code reviewed. We have found competence problems with new hires relatively quickly by having code reviews and more importantly by how they responded to the code review. We have learned what things that seem perfectly clear right now in the thick of programming that section that will not be clear in maintenance. This is invaluable. It may be that the only thing needed is a comment as to why something was done. We have found some fundamental misunderstandings about our database design that needed to be corrected for a report to actually have the correct information.
Often what I have seen in a code review is that, by the very act of explaining something to someone else, the developer will have a lightbulb turn on in his head and realize that there is a bug the reviewer didn't see.
And boy can code review identify those cowboy programmers who won't follow any standard or use mandated tools and whose code will be nigh onto unmaintainable by anyone else. And it can force them to get with the program or get out too.
The people most resistant to code review are often the people the organization most needs to get rid of because they know in their hearts their code can't pass a code review.
Agile guy would say: "You don't need a code review, just do pair programming and write good tests" :)
Code review is a good way of spreading knowlege and good practice through a team. In my experience it is a good idea to make sure all code gets reviewed, and try to vary who reviews what code.
In my current team everyones' code is reviewed equally, and both programmer and reviewer have to be satisifed with the code before it can be released. This applies just as much to senior developers being reviewed by more junior developers, a junior developer reviewing another, or other senior developers reviewing each other. If the reviewer is inexperienced or does not feel comfortable reviewing any particular piece of code then they will team up with another developer (and maybe also the original developer) to do the review as a group.
I've been in the business for more than 20 years, working for software companies and for different businesses and none of these places had a code review process. However, I can understand and appreciate the benefits of having one. Essentially, as I understand them, they should be used to ensure adherence to standards, which should be followed in order for others to be able to more easily maintain the code in the future. Code readability might also be checked in a review process as this too will ensure that maintenance can effectively work with the code.
Currently, I work in a small shop where I am the single developer. Occasionally we have brought in contractors to help with the backlog. At least one or two of those contractors have written code that didn't necessarily meet mine or the companies' standards, but it did work well and was at least somewhat understandable. When I pointed this issue out to management, they didn't care, they just wanted to know if it did what we paid them to do. So, I guess it just depends on the company, and whether or not clean, easy to maintain code is the desired product, or if they just want something that works well for the money.
Obviously as a developer, and one who has to maintain code, I would like to work with clean code that follows a standard of some sort, but I dont always have that luxury, so I make the best of it and deal with what I have, even if it means sometimes having to re-write some code in my own standard.
Code reviews can identify defects earlier in the software lifecycle when issues are easier (and cheaper) to fix. At SmartBear we have developed a peer code review tool and have also done a lot of research into how to make code reviews effective. Based on data from our customers, defects found in code review are 8-12X cheaper to find and fix than defects found in QA. The cost savings alone makes code review worth it, but there is more value than just that. Code review is an excellent way for everyone on the team to learn and improve as software developers.
There are some traps that can cause code reviews to become less effective, and it seems like your organization is caught in one of them. Make the code review about the code, not the people. Titles mean nothing in a code review. Appeals to authority (you must do it my way because I'm the team lead) do more harm than good. Teach instead. Senior engineers should explain why it should be done their way, not just demand that it is. If you're having a hard time explaining a concept, it's a learning experience for you too. Both of you will be better developers for the effort you put in.
I think code reviewing ALL CODE is overkill. The amount of time it takes to review all code could be better spent elsewhere. Alterntivley I think critical code and or particularly complex pieces need code review but certainly not every line of code.
In my opinion, code that is going to be used by a company, whether it was written by a Junior or Senior developer, should always be reviewed. Why? Because what if the code had several bugs? And what if, during the time this code was being used, the program crashed? To stop these things from happening, all code should be reviewed before use.
But what about those companies that don't review code? They're probably the companies that have lots of tech problems and lots of, as they tell consumers, "crashes" ;-).
So let me answer all of your questions:
Code Review: The Code Review process must be a vital one for all, I will explain who are all get benefits due to conduction of code review as well as what are the benefits they are getting.
1. Benefits Obtained By the Company Due to the Conduction of Code Review: If frequent code review is conducted, then the company can get the final products in much better optimized way which help them to get branded name in their market and also help company to get or improve their current CMMI level.
2. Benefits to Team Leader Due to the Conduction of Code Review: As we all know a teacher can easily identify the mistakes, because they are reviewing the answers of their students more frequently, so they can get an idea , in which areas there may be possible for wrong things. similarly the team leader also knows what are the things goes wrong in this areas. How can we rectify those.And also Help the team Leader to grab the news ideas from the junior developer too.
3. Benefits to The Junior Developer Due to the Conduction of Code Review: Junior Developer can easily grab the ideas about Code review Process, also they are able to get what is the coding standard, For Instance: to create API in a proper way, they will learn the standardization of the coding which may help them in future specially when they are become in Higher level post.
So My Conclusion is Code review is a Very Very Vital Process for all[even for team Member too], since code Reviewing helps us to correct our careless mistakes in our code,because we are all human beings, so we cannot predict that we never do careless mistakes in code.
What is the difference with interfering with your ideas before checking in your code(review) or afterwards due to a bug, getting too clever/difficult to understand, or not following accepted standard practices? Is it your ego?
You can't disreguard the merits of code review or anything else just because it is being implemented poorly by a less qualified team member you don't respect. Code Review isn't some highly complex process that only a few super programmers are capable of grasping. I'm not sure there are many programmers or professional writers who are capable or have the time to self-edit.
Have you ever returned to a line of code a few months later and wondered what was I thinking? There would be a better chance of catching it with a code review. You just caught it and you're only slightly better than the programmer you were awhile ago - I hope.
IMO a code review should be essential for all developers, but only when the people doing the review are competent themselves. In the past I've had code rejected in a review because, I kid you not, I followed SOLID, did some Dependency Injection and had the code organized into namespaces and folders according to the logical design, and included a small suite of unit tests to verify the code. The code was rejected as "too complicated" and I was told to use one class that smushed everything together and remove the tests because it wasn't how the company wrote code.
A code review like that is worthless, but a code review with a competent team can often illuminate something about the design (e.g. Why you should do X and Y but not Z) or indicate an actual flaw (e.g. Doing X will cause Y to fail for incorrect reasons).
Of course Code Review isn't needed. Then again, neither are tests, continuous integration, source control, customer involvement, profiling, static analysis, decent hardware, one-click builds, bug tracking, the list goes on.
Along with Code Reviews, the things I mention above are tools that help ensure software quality. With a combination of skill, luck, time and determination; you can deliver quality software without any of this stuff, but it's more likely that you won't.
In your scenario, there is nothing to be confused about. Not every organisation indulges in every best practice. They may disagree with it, it may conflict with a different best practice that they do implement, or they may consider that the overhead of implementing it is too great for them at this point in time. Depending on their circumstances, they may be correct in doing so, or they may be making a false economy. For some tools, (e.g. source control) the payback/effort ratio is so good that using it is a no-brainer; for others it's less clear.
There is no doubt that code review is a practice that introduces a significant overhead. Because of this, organisations will seek to minimise that overhead, either by not doing it at all, or by only doing it in certain situations (e.g. for a junior team member, or a particularly hairy change). It is not always obvious that it pays back more (in catching bugs, reducing technical debt or sharing knowledge) than it costs. Most of that payback is difficult to quantify, whereas its very easy to count the number of man-hours your organisation spends doing reviews. The easiest bit to quantify (reduced bug count) is easy to attribute to other factors (e.g. "of course it has fewer bugs, it's more mature").
We make online football game in Turkey. A lot of users and game masters helps us on functionality. Also they give comments on needed features. So I think, if you have a lot of users, functionality tests can be done for helping or getting badges. Collobration of developers, game masters and users with forums, support teams and private test environments create a social project.
Sure code reviews and sharing experiences between development team is needed but if it's not critical you don't have to force yourself.
I think that how verbose 2nd party inspection is depends on your lifecycle, whether you're more agile, or more waterfall in your processes. I think it's reasonable to do high level designs/inspections, as well as more detail level design inspections. I think it's also good to involve several members of a team to inspect.
They're absolutely necessary since they have little experience.