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

Questions tagged [code-reviews]

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

8
votes
4answers
326 views

How to make better code reviews when pull requests are large?

Disclaimer: There are some similar questions, but I didn't find any which touch specifically the problems you face while reviewing a large pull request. Problem I feel my code reviews could be done ...
3
votes
3answers
119 views

Are there any drawbacks to using a nested class instead of declaring a new one?

I'm doing code review on a change my co-worker made to our Java application, and I've found something I'm not very familiar with - a nested class. From reviewing the code, it seems like the nested ...
0
votes
1answer
75 views

How to handle complete rewrite of a file in regards to code review

My situation is this: I am contributing to a Symfony/PHP project. I had to completely reskin one page in the application. The controller logic is almost totally unchanged. I only had to change the ...
4
votes
3answers
195 views

Making pull requests easier to review while practicing XP and refactoring

I practice TDD and refactor aggressively. Recently some colleagues have complained that the resulting pull requests are difficult to review, or would require a lot of regression testing. When I ...
53
votes
11answers
10k views

Agile Practices: Code Review - Fail the review or raise an issue?

At the end of a 2 week sprint and a task has a code review, in the review we discover a function that works, is readable, but it's quite long and has a few code smells. Easy refactor job. Otherwise ...
2
votes
1answer
100 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 ...
10
votes
5answers
1k 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
41 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
658 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
791 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
698 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
66 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
151 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
993 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
297 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
720 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
515 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
971 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
178 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
308 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
953 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
701 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
391 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
89 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
202 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
272 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
679 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
947 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
273 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
328 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
612 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
743 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
872 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
552 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
572 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
327 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
691 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
855 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
428 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
717 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 ...
176
votes
19answers
23k 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
93 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
198 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 ...