Stack Exchange Network

Stack Exchange network consists of 174 Q&A communities including Stack Overflow, the largest, most trusted online community for developers to learn, share their knowledge, and build their careers.

Visit Stack Exchange

This tag is for questions about the practice of code review and code walkthroughs. For reviews of existing, working code, please see http://codereview.stackexchange.com

1
vote
1answer
89 views

How to review hundreds of commits and formalize merge request(s)

At my work every line of code must be reviewed and this review is formalized in a merge request. Now we are a facing a situation were one developer developed a prototype for a new product which is ...
6
votes
5answers
695 views

Clean Code - Should I change the variable 1 to constant?

To avoid magic numbers, we often hear that we should give a meaningful name for a variable. Such as: //THIS CODE COMES FROM THE CLEAN CODE BOOK for (int j = 0; j < 34; j++) { s += (t[j] * 4) / ...
0
votes
0answers
35 views

How to design parallel web api in c# .net 4.5?

I am trying to design a web api that can get data from an external server but with limitations. I'm trying to figure out how best to design it to be efficient. My api has an endpoint that takes an ...
2
votes
6answers
530 views

How to avoid code duplication in if else condition?

How can I avoid duplicate use of doSomethingCommon() in the following block of code? doSomething(); if (conditionA) { doSomethingSpecificToConditionA(); doSomethingCommon(); } else if (...
13
votes
5answers
729 views

Getting ALL developers do code reviews

I'm a software developer in a 7-8 developers team. We've been doing code reviews for quite some time now and code quality has improved over time. However I recently noticed that some developers are ...
2
votes
4answers
685 views

Is “Let more people able to review” a reason to “write c++ as if Java” at some degree?

According to Should we avoid language features that C++ has but Java doesn't?, I know it is horrible to write C++ as if Java, mostly because it drops the beneficial features of C++ languages. But ...
-1
votes
1answer
62 views

How to Gain full Confidence in Pull Request builds

Having confidence means Pull Request builds (in Jenkins) will finally allow you to merge the code after a Pull request builds successfully and is reviewed. You're 100% confident that it can go into ...
0
votes
4answers
144 views

Magic __init__ and regular init methods in a python class

Is naming a regular method init only confusing or is it an error? class A: def __init__(self, x): self.x = x def init(self, y): self.y = y**2 def reset(self): ...
0
votes
1answer
716 views

Easily exclude commits on a merge based on some condition

My eyes are destroyed. I am using a screen reader. We are using git as our version control system on our project. We host our own server and use ssh to access it. We put each module under separate ...
2
votes
4answers
289 views

Why chain assignments to clear variables?

Why do some developers in C for example chain together assignments of variables like this: i = j = k = 0; or sometimes formatted like i = j = k = 0; Is there some underlying complier optimization ...
22
votes
6answers
6k views

How do I document my code for minimum time review? [closed]

I want to document my code such that there is minimum need for reading and browsing the code again months later. I know that there are different types of documentation (in source code and outside, ...
0
votes
2answers
373 views

changing the value of String object in java

I have to set the value of a string object "result" depending on the results of different methods and different if/else conditions. In the end, there would be one (last value set) in the string that i ...
10
votes
4answers
496 views

How do I wrap a service so it is simpler

We have a dependency to a third-party service which exposes a gigantic interface of which we only need like 3 methods. Additionally, the interface changes frequently... I've decided to wrap the ...
11
votes
3answers
844 views

How to give feedback after code review process

I am currently reviewing some of the code of junior developers who just joined my team, I am wondering how should I deliver the output of this review: Should I fix the code myself? Should I give them ...
4
votes
1answer
175 views

What's a reasonable in-house code audit policy?

In a company of about ~100 developers, and ~2 pen-testers, a product is being developed. There's a good amount of diverse code coming in via github PR requests every day. The goal is to review as ...
3
votes
3answers
305 views

Can you consider a bug fixed when the underlying cause was not fixed?

Let's say there is an extensible system in place, which uses an algorithm that requires that the elements it operates on are unique. The signature to call this algorithm looks like this: function ...
23
votes
4answers
914 views

How to handle a TODO in a pull request?

When I review the changes in a pull request, I sometimes stumble upon a comment with a "TODO" note which may be there for different reasons, in our case mostly because of: the solution used to solve ...
2
votes
3answers
635 views

two level code review using git pull request in BitBucket

The following procedure: We have Development branch and Developer create feature branch from Development Work and commit in this branch Rebase from Development Create pull request Code review Merge ...
5
votes
5answers
390 views

Check if variable is not null before setting to null

One of our team likes to write code like this (C#): if (_someVar != null) { _someVar = null; } Rather than simply: _someVar = null; I've never used the former as I always assumed it was ...
0
votes
3answers
251 views

What's an effective strategy to mark code as reviewed?

I am reviewing a big C# code base and thought on how to mark it as reviewed when it is, in order to not review it twice. Currently I have a ReviewedAttribute which I use to decorate members that have ...
1
vote
1answer
87 views

Benefits for the developer of separating pull requests by logical change?

I am currently responsible for reviewing one of my team mate's code for a front end web app. We have established a workflow which involves creating a feature branch in git for each feature we define. ...
4
votes
1answer
200 views

How to make sure that each field's setter executes a specific code?

I am working with Java 1.7, IDE is Eclipse Indigo. I have a base class which has id as an int. It also has a boolean variable onlyIdInitialized; I am setting it to true when an object is created with ...
2
votes
3answers
268 views

How to deal with people not finding the time to do code reviews?

I'm finding that others not having the time to do reviews in a code-review enforced open-source (so volunteer) project is really slowing the velocity - it's really unmotivating to have 3-4 PRs of your ...
15
votes
5answers
660 views

What is a helpful mindset when conducting a formal code review

Our team has recently started conducting code reviews against each checkin. As the team lead I'm trying to find a balance between providing too many suggestions, annoying developers and decreasing ...
10
votes
4answers
941 views

Reworking a function returning an integer code that represents many different statuses

I've inherited some awful code that I've included a short sample of below. Is there a name for this particular anti-pattern? What are some recommendations for refactoring this? // 0=Need to log in / ...
5
votes
4answers
272 views

Is it acceptable to assign a class variable within a method?

I've just done a code review where I came across a method like this: private void FindMinAndMax(double[] values) { // Some algorithm that results in two class vars being assigned: _min = ... ...
6
votes
3answers
327 views

My Company's Private API as My Portfolio

So I've been developing an API for the company I work for for a year. Recently, I plan to move to another workplace and have done an interview with the recruiter. She asked to see my GitHub page for ...
1
vote
4answers
579 views

Should I comment or remove any un-used code from my Solution?

I was working in a story and in last minute, I have been asked to hide something from UI and we will used it next release. Should i remove it or comment it Should i remove or comment anything ...
92
votes
11answers
11k views

In code reviews, should the reviewer always present a solution for issues? [closed]

When reviewing code, I normally try to make specific recommendations on how to resolve the issues. But owing to the limited time one can spend for reviewing, this does not always work well. In these ...
4
votes
7answers
734 views

How can wrapping an expression as a function be Clean Code?

Another programmer just started working in our team and submitted a patch. What required was to have something that compares and check a couple of conditions and set a property based on the outcome. ...
6
votes
7answers
784 views

Does code-review typically involve UAT/QA testing?

Does code-review typically involve UAT/QA testing? We are trying to establish a meaningful code review process and we have come to a disagreement among our development team. Some of are holding that a ...
3
votes
3answers
518 views

Single Responsibility, Api Methods, Error Logging, and Helper Classes

I'm getting in a heated debate about the Single Responsibility Principle and some code that I wrote. I feel strongly that I am right and would like some unbiased feedback. Basically, we have decided ...
10
votes
1answer
570 views

Who can review my code if I'm the only one that knows the technology?

I'm currently working on introducing React in the company I work for. As the only one having done courses on the topic, who can I ask for a review to make sure the code is following 'best practices' ...
186
votes
22answers
43k views

How do I handle disagreement in a code review regarding an unlikely edge case?

I am working at a robotics startup on a path coverage team and after submitting a pull request, my code gets reviewed. My teammate, who has been on the team for more than a year, has made some ...
3
votes
5answers
306 views

Is consequent and efficient refactoring enemy of code reviews in a legacy code context?

There are some Tech Leads that perform code reviews for each Git pull request. Apparently, a very good practice to be aware of the evolution of code. Basically, they expect to inspect a few lines of ...
1
vote
1answer
661 views

Console application for developer interviews [closed]

I have been tasked with helping our HR department create some new interview questions for candidates applying for development positions. As part of the process, I would like to assess their ability to ...
17
votes
5answers
849 views

How to do efficient code review during release fever?

The deadline for a release is tomorrow, your collegue finally finished his task that's crucial for this release, project manager is standing over your shoulder and presses you to finally make a build ...
2
votes
3answers
411 views

Is Professional Code Review/Mentoring Offered? [closed]

I'm a newer developer who has worked on some personal projects as well as non-profit/charity projects. However, I seem to be the most "senior" developer in my circle, meaning, most guys come to me for ...
115
votes
8answers
12k views

What to do when code submitted for code review appears to be too complicated?

The code is difficult to follow but it appears to be (mostly) working well, at least with superficial testing. There might be small bugs here and there but it's very hard to tell by reading the code ...
13
votes
4answers
1k views

Code review workflow where pull request author must merge

Several teams at my company practice a code review workflow I've never seen before. I am trying to understand the thinking behind it, with the idea that there's value in making the whole company ...
13
votes
4answers
691 views

How to encourage junior developers to participate in code review?

I'm currently working as a senior dev with 3 juniors below me and have introduced a code review process to help managed to quality of code going into production. I feel its very beneficial for all of ...
174
votes
19answers
22k views

How to find positive things in a code review?

After some serious quality problems in the last year, my company has recently introduced code reviews. The code review process was quickly introduced, without guidelines or any kind of checklist. ...
3
votes
1answer
90 views

Should security code review involve stylistic culprits?

I have worked in company A for less than 1 year and I was given the task to do a security code review for a small project I was involved in the late 3 months. Until now I have been mainly been reading ...
0
votes
1answer
193 views

Code review and then what? [duplicate]

In my team we have a policy that each code should be reviewed by at least one other developer. When the review is done the reviewer just clicks the merge button so the code ends up in development. ...
25
votes
5answers
1k views

How to gradually introduce code reviews?

I'm lead on a team with a half-dozen senior engineers. I very much believe it would benefit us greatly to do code reviews for all the standard reasons. Not necessarily every change but at least a ...
3
votes
1answer
174 views

Code Review Best Practices for New Applications/Large Changes

I see a lot of advice lately about making code reviews more granular, and as small as possible. For example, a lot of recommendations say "no more than 400 lines of code for a code review". This ...
1
vote
0answers
53 views

What are the boundaries of the term 'Consistency' in the context of system's design?

In many books on code style or design of the software systems authors often stress out the importance of being consistent, yet they do not give a precise and clear definition for it. Now I need that ...
2
votes
0answers
36 views

Using gerrit(-like) tool to ensure high bus-factor

A nice side-effect of using reviews is that it will ensure that other programmers have some knowledge of the code that is being merged to the project. The more that reviews changes to the project the ...
18
votes
5answers
1k views

What should I do as a reviewer when code “has to be” bad?

I work at a badly designed, overengineered project where mandatory code reviews have been introduced few months ago. I've been asked to review a substantial piece of code that implements a new ...
142
votes
16answers
26k views

What do you do when code review is just too hard?

OK so a lot of code review is fairly routine. But occasionally there are changes that broadly impact existing complex, fragile code. In this situation, the amount of time it would take to verify the ...