Tag Info

Hot answers tagged

172

Code with spelling and grammar errors is unmaintainable. People won't remember the bad grammar, so they'll try to call the function as it should have been written, and that's how bugs happen. You can't grep for something in the code if you don't know how it's spelled. Most people who make grammar/spellings do so inconsistently, so they'll introduce many ...


143

Reviewers should be objective. It's clear that you've formed an opinion about the code in question before you've even reviewed it, and it sounds like you and the fixer have staked out positions. If that's so, then you're going to have a difficult time appearing objective, and an even more difficult time being objective. None of that helps the process, and ...


103

Double-check your motivation. If you think the code should be changed, you ought to be able to articulate some reason why you think it should be changed. And that reason should be more concrete than "I would have done it differently" or "it's ugly." If you can't point to some benefit that comes from your proposed change, then there's not much point in ...


73

Should junior programmers be involved as code reviewers in the projects of senior programmers? Yes they should. It is a good learning experience to read other peoples' code. (And that applies both for good code and bad. Though one would hope that a senior developer's code wouldn't be bad ...) Obviously, it is unwise to only have juniors doing the ...


61

So my code is late too. No, it is not your code, it is the code of you and the senior. You are working as a team, you have a shared responsibility, and when you two miss a deadline, it is the fault of both of you. So make sure the one who makes the deadlines notices that. If that person sees that as a problem, too, he will surely talk to both of you ...


54

This seems to be a pretty common prevailing attitude among some developers. Everyone seems to feel that a code review is some challenge to their work, and that makes no sense to me. A code review is a quality assurance mechanism that has the added bonus of education to go along with it. We implement code reviews extensively where I work, and I've fostered ...


51

The primary purpose of a code review is to find defects or potential problems. The required participants in the review should be the people who are best suited to identify these problems, regardless of their title or seniority. As an example, if an application is being developed in Python and the junior engineer has more experience with the Python language ...


50

Code reviews are a great practice. It is probably the best way to learn from mistakes and to see how certain problems are solved by others. It is also one of the best way to maintain quality in a code base. Code reviews happen in many companies, though it is difficult to say that there is a specific process that they all follow. In more formal code ...


46

Since it's not clear from your question, I just want to point out that a gatekeeper workflow is by no means required with git. It's popular with open source projects because of the large number of untrusted contributors, but doesn't make as much sense within an organization. You have the option to give everyone push access if you want. What people are ...


46

It's important to highlight positives as well as negatives. I know if I were reviewing the refactor of a particular hellish subsystem into something neat and clean, I'd probably buy the programmer a pizza for his efforts. If you're using reviews as training, it's doubly important - highlighting a good piece of code will be helpful for the junior programmers ...


39

A formal code review could go like this: Programmer sends material to reviewer Reviewer goes over material Programmer and reviewer sit down go through the material. Programmer takes notes. Design discussion are postponed. The programmer corrects whatever came out of the review. The programmer should always talk to the reviewers, otherwise way too much ...


39

Improve Quality and Morale Using Peer Code Reviews http://www.slideshare.net/SmartBear_Software/improve-quality-and-morale-using-peer-code-reviews Things Everyone Should Do: Code Review http://scientopia.org/blogs/goodmath/2011/07/06/things-everyone-should-do-code-review/ Both of these articles state that one of the purposes of code review is to share ...


38

Write down your reasons for rejection and defenses for likely counter arguments. Then discuss rationally with the fixer and if necessary escalate to management. If you have sufficient documentation (including code listings, test results, and objective arguments), you'll be well covered. You will cause some ill will with the fixer, so be sure the issue is ...


36

I have worked at a job where check-ins were limited to team leads only (and team leads couldn't check in their own code). This served as our mechanism to enforce code reviews, largely because of a number of incidents where bad commits got into the codebase even around gated check-ins and static analysis. On one hand, it did it's job. A number of bad commits ...


34

Unless the project is aimed for developers (eg: a development framework, in which case you WANT them to criticize it if it makes you learn even more), you shouldn't worry. But even then, there are many open source projects aimed for developers that are crap, yet people love them because they go to the point (think of Codeigniter, which is very poorly ...


34

In my estimation, clean code has the following characteristics: It is clear and easy to understand, It is loosely coupled, It has relatively low cyclomatic complexity, It has a sensible, flexible architecture, It is easily discoverable, It is well-designed and It is testable. Note that these are all subjective, although principles like SOLID and ...


32

Recently in LinkedIn news, there was an article about conflict resolution in the workplace and being humble, rather than appearing arrogant. Sadly, I can't find the link now, but the general gist was to try to form questions in a non-confrontational way. Please don't take this as me implying that you are arrogant, it's just the way in which the article was ...


31

I have been in your shoes and I don't think there is any easy solution. Paying a consultant to look over your code is not a good way to spend money. If your problem is that you feel lonely and don't have anyone to talk to about programming then I can't help you there but if you are really interested in improving the quality of your code then the best thing ...


31

Sounds like instituting some sort of code review policy might be beneficial on multiple levels. Some immediate benefits: You can directly influence the quality of his code before code is committed thus keeping the code base quality high Keeps you from making similar mistakes that another set of eyes may catch In the absence of coding guidelines, reviews ...


30

I would add that if a "Junior" programmer can not understand a seniors code then that in itself is a good measure of the code. OK there may be times when it just isn't possible to write code that everyone can understand, but hopefully those are exceptions - if only 1 or 2 people can understand the code then what happens when those people are not available ...


29

Feature envy is a term used to describe a situation in which one object gets at the fields of another object in order to perform some sort of computation or make a decision, rather than asking the object to do the computation itself. As a trivial example, consider a class representing a rectangle. The user of the rectangle may need to know its area. The ...


28

No. Anyone should be able to commit. If you have problems with bugs being committed it's not the source control policy which is wrong. It's the devs who fails to make sure that what he/she commit works. So what you have to do is to define clear guidelines on what to commit and when. Another great thing is called unit tests ;) There is an alternative ...


27

There is a chance that you're being singled out as a woman, but it's also possible that you're just a junior developer and new to the job. Error-checking and expressive messages are important. If you're going to add something to the code, make sure it's right and up to the team's standards. Similarly, if you're modifying someone else's code, try to improve ...


26

Don't point them out as defects in a formal code review. Instead, mark up a listing and talk with him/her PRIVATELY about them. Be as diplomatic as possible about it, just "Hey, something I noticed, and I've run into people who REALLY look down on this kind of thing, they think it makes the programmer look careless and sloppy." If this is code a customer ...


26

When I do code reviews I tend to just have a running monologue, so as I'm making sense of what I'm reading there will be a lot of "Ok, I see what that does.. Good it connects to this and calls that, alright.. and that piece depends on both of those alright.". I think in this way it's not "oo la la this is so great!", it could be perfectly trivial boring ...


26

In a good team, you should have a queue of development tasks assigned to you in an issue tracker. That way, while you are waiting for a reviewer, you could (should) work on next task waiting in that queue. Once you get used to work in that fashion, this will open an opportunity to have your changes reviewed in "batches", thus decreasing delays. If you ...


24

I think you just have to keep doing what you are doing. Have a clear set of coding guidelines, and enforce them during code reviews. If a developer gets 50 or 100 lines of "Add a space here" and "Spell 'iterator' correctly" every time he tries to check something in, and he is actually not allowed to check in before all of those get fixed, eventually he'll ...


24

As this is code for an interview the candidate is probably going to bring what they feel is the best representation of their work. Before delving into the code, ask them about it. Get them to demo it if they can. This helps you get an idea of what they have built and allow you to start picturing how you would have done it. Next, move to looking at the ...


24

As a general rule of thumb, open sourced programs have three groups of people who look at the source code. People who are considering modifying the code to make the program work slightly differently for them, to port it to a different platform, or as a jumping-off point for their own programs. If they don't like the code, they typically just won't use the ...



Only top voted, non community-wiki answers of a minimum length are eligible