2

When working with other people in a project, is it acceptable to refactor some of their code?

I'm working with another developer and I notice a lot of duplicate code. To make things easier for myself, I will locate the duplicate logic or notice a similar pattern that can be used. I will then put the repeated code in a loop, DRYING up the code.

I work with Rails, so DRY is an important principle. I will also make trivial edits on the code. For example, I would change things like

unless @user.nil?

to

if @user

I'm working on the project as well, and I find repeated blocks of code really annoying, so I take the lead to refactor his code. Is this acceptable?

DrunkDog
  • 37
  • 2
  • 1
    possible duplicate of [How can I tactfully suggest improvements to others' badly designed code during review?](http://programmers.stackexchange.com/questions/113593/how-can-i-tactfully-suggest-improvements-to-others-badly-designed-code-during-r) – gnat Feb 11 '15 at 15:25
  • I don't know if I'd say this is a duplicate, the referenced question is talking about suggesting improvements while this is talking about making improvements without consulting others. – sydan Feb 11 '15 at 15:32
  • Do you practice collective code ownership? Is there a style guide? Are there tools that enforce the style guide? –  Feb 12 '15 at 12:43

2 Answers2

7

Yes, but I recommend changing how to go about doing it.

Rather than refactoring on your own, do a code review. Work on refactoring and cleaning up the code as a group. You clearly know there are better ways to write the code, so share! Maybe your coworker will learn and start writing better code. In the future, maybe you will not need to spend so much time improving his code.

The key here is to mentor your coworker. Most software is built by a team. There is more to a team than simply having each person do his own job in isolation. Collaborate, work together, and help each other to become better developers.

6

Short answer: No.

Longer answer: If your refactorings are truly improvements, then you're engaging in Codependent behaviour - you're supporting your collaborator's bad habits & not giving him/her the opportunity to improve.

If your refactorings are trivial, then you're probably wasting your time and/or risking the possibility of falling into an edit loop where you refactor the code to A, they refactor it back to B, then you fix it back to A, and so on.

At the very least, talk to your collaborator at least once about this issue.

Some folks may improve their practices and thus solve the problem. Others may use you as a safety net, and continue to write sloppy code (or worse, get even sloppier knowing you're backing them up). A few immature souls may take offense at you changing 'their' code.

Dan Pichelman
  • 13,773
  • 8
  • 42
  • 73