Coworker thinks code review cost too much time

Question

Almost a year ago I switched to my current job. I have 2 prior years of programming experience but this is my first time working on an entirely new product. I am part of a small team of (5 or less) developers. I have 1 coworker programmer with whom I sit in the office. My coworker who is a Sr. Developer is also project lead, and is therefore responsible for the product specifications, schedule and the expected release date.

Whenever I request a code review, the Sr. Developer claims that it's pointless to do so, unless the regarding feature has been finished. The Sr. Developer is convinced that because of our tight schedule the code reviews would consume too much time, and are too much out of context, because the feature isn't finished yet.

However, I keep making (logic) mistakes of which I am not aware, until the code is reviewed. This is because I'm still unfamiliar with some of the concepts used. These concepts are not pointed out until the code review is done. This means I usually spend alot of time refactoring the code, sometimes tossing days of work.

I've requested to schedule a moment once a week for code review, in hopes of preventing tossing days of work. The Sr. Developer isn't in favor of that idea and claims that he wants me to function less dependently of their input.

Right now I'm looking for insights from Intermediate or Senior programmers, hence I've some questions below. Feel free to not answer the questions and give other input instead, all input is appreciated.

• Am I focusing too much on the standard coding conventions?
• Are code reviews on a frequent base required?
• (If applicable) how frequent should code reviews be done?
• Why should(n't) a code review be required on a frequent basis?
• When in the process is the best time to do a code review?
• How would you deal with this situation if you were me?

Thanks in advance for your input.

EDIT: Thanks everyone for your input. It helped me look at the situation in multiple ways. I've had some talks and as it turns out, there are no specifications to work towards. This is why code reviews are useless because we're not sure we'll be using the code after all... I'll leave it at that.

Answer 1

Have you considered a job change? Seriously feedback like code-reviews is vital to YOUR career growth. Some companies take code reviews seriously, some don't. Even if you could manipulate this situation to force the senior-dev to review your code, he's not going to review it to the degree that you want since he feels this is a waste of time. Rather than trying to influence up the chain-of-command, find a position where the feedback-loop is working as it should.

Answer 2

Am I focusing too much on the standard coding conventions?

One could argue that if, after a year, you are still making logical mistakes that you cannot find yourself before code review, that you are not focusing enough on standard coding conventions. Do you do any unit testing? What kind of testing do you do? It is general good practice to verify your code before submitting it to review.

Are code reviews on a frequent base required? (If applicable) how frequent should code reviews be done?

I'm not sure they're "required" per se. Code reviews should be done as necessary, not just for the sake of doing them. If you are making incremental changes on a features/development branch, then you should perform most of the code review yourself and only involve others when you have reviewed it to the best of your ability.

Why should(n't) a code review be required on a frequent basis?

Code review is very time consuming. I have previously spent a solid week reviewing code for someone who would not self-code-review at all. We were at over 30 patches before the code was finally acceptable. At the same time, I had other things I was expected to be doing.

When in the process is the best time to do a code review?

When you have verified your code thoroughly, and done a self review. Make sure people aren't wasting time commenting on styling errors.

How would you deal with this situation if you were me?

Do more self-review. You should go through your code and verify the logic before you ask anyone else to review. Test test test everything. Write unit tests, check your logic using Python (great for validating corner cases!). Hold design reviews before jumping into implementation if you frequently find yourself not understanding the logic behind an implementation.

Answer 3

In nearly all cases when designing a business process, or optimizing one, it's best if the means are justified by the end goal.

You need to know what the end goals are for your team: likely, it's something obvious like delivering code of a certain quality on a certain timeline. But it's important to not assume, since in many cases there's subtle nuance in terms of what counts as "good enough" or "ready." Once you have that clearly understood, you can test any process or suggested change against that goal. Will "more code reviews" actually help you get to goal X? Having X well defined - including nuance and subtlety - makes that a very easy question.

If it turns out that you think adding more code reviews can actually help, then having gone through this thought process should give you the objective justification you need to help your coworker understand your case. Instead of just sounding like a needy newbie, you can show your coworker that helping me understand my mistakes at an early stage in development will help us, together, reach our goals. Or whatever specific sentence is appropriate. Your coworker may see a few hours a week spent in code reviews as wasted time, but if you can objectively show that doing such reviews actually saves time - and therefore, makes delivery deadlines easier to achieve, then you should have an easier time getting the change you want.

It may also be the case that you're able to identify a problem (you are making logical mistakes) but your idea of a solution (more code reviews) ignores other potential options for solving the problem. So, while you may be able to relate your problem to your team's goals, you may also want to approach this by presenting the problem instead of your solution. This gives others, who may have more experience in software development processes, the ability to guide you towards a better solution.

It's important that you approach this objectively. It's easy, and common, for newer programmers to want to over-optimize things, and over-compensate for perceived issues or problems. People that fall into this trap can find themselves striving for a sense of perfection that ends up having no meaning in the real-world context in which they're actually working.

In a vacuum, "more code reviews" might sound like it's always a good idea - but, practically speaking, it is quite possible to spend so much on review that your overall goals are missed. In other words, your coworker may have a legitimate point.

You listed a handful of questions - ultimately, we can't answer those in a way that's ideally applicable for your situation. However, you can do some reflection on your situation, and come up with the answers yourself.

Answer 4

Code review is similar to hygiene... Do you take 10 showers a day or once a week? The answer depends on your environment... anything over the required is a waste of time. The trick is to find out what is required in your environment...in some development shops it’s before you push anything to source control, in some shops, it’s pick out a few commits after things get into source control.

Try pair programming if you’re really wanting 100% code review 100% of the time. I think that’s a little extreme but some shops/developers swear by it.

Answer 5

I kind of agree with the point of

pointless to do so, unless the regarding feature has been finished.

Logical mistakes not suppose to be a reason for code review, there are specs and QA to catch that.

But, IMHO, your tasks may be broken down in to larger than logical (unmanageable) pieces of work (tasks).

Are you guys using TDD?

Answer 6

I would focus on making your pull requests easier to review. I review several per day. In the best case, I can complete a review in under 5 minutes, while I'm waiting on a build or something so it doesn't interrupt my work. In the worst case, it's a half hour per day for several days until it can get merged, and I have to specifically allocate time to address it. Thankfully, the first category is much more common. What makes the first category so much easier to review?

• It makes a minimal change. Our latest big feature required upwards of 30 pull requests to complete. Other teams sometimes do the same amount of work in one or two pull requests, but then take weeks to get it up to standard to merge it.
• It is thoroughly tested. The vast majority of my comments on easy code reviews are asking to add a test for a particular use case.
• There aren't any design surprises. We've already had design discussions over chat or at the whiteboard. I don't have to tell them they have been going in the wrong direction for the last week.
• There aren't any apologies. If you know I'm going to complain about something, don't apologize for that code, just fix it. If you don't know how to fix it, hit me up on chat.
• It clearly denotes what will be coming later. If your pull request is just laying runway and not actually finishing the feature yet (as should frequently be the case), make that clear in your pull request description. This is different from an apology. Apologies are for something bad quality you don't intend to fix. This is for something good quality but incomplete, with a defined plan for completion.
• It provides context. I'm a good reviewer because I'm familiar with the domain and the language. I'm usually not as familiar with your exact feature request as you are. Provide the context I need to get up to speed.
• The reviews are spread around. It shouldn't be just one person doing all the reviews. Peers new and old can all provide valuable feedback.
• It has been refactored. I can tell when people have made refactoring passes and when they haven't.
• It shows a willingness to accept feedback. If you argue about every suggestion I make, forcing me to write lengthy justifications, it's going to take me a lot longer to get the time and motivation to finish the review.
• It was not done blindly. If you don't understand how the code you're changing works, don't change it yet. Gain an understanding first. Lengthy code reviews have a lot of comments along the lines of, "This code doesn't work the way you think it works. Here's the background you need to do it right."

Answer 7

The Sr. Developer is convinced that because of our tight schedule

Right there is what you need to focus on, not code reviews or best-practice. If Sr. Dev is not reviewing your code, it is doubtful that any other developer is reviewing his.He is getting whipped from above. And no, self code-review does not work. No-one wants to see the ugly in their own code, especially when rushed.

once a week for code review, in hopes of preventing tossing days of work. The Sr. Developer isn't in favor

Where is the continuous-delivery? Why are tasks not being broken down into at most one day deliverables?

What can you do?

• Document request, CYA.
• TDD - test driven development. This will alleviate much of your apprehension, allow you to understand the problem finer, and hopefully break the feature apart.
• Break the feature down into smaller features of one day deliverables.
• Very difficult - try to show Sr. Dev how code review can loosen the tight schedule by reducing rewrites. See if their are tasks/features Sr. Dev can be offloaded, to you.