views:

412

answers:

16

Do you do it when you’re in the code doing something else?

When your manager approves it? (Seems this never happens)

I guess some of this depends on the impact of the changes. If I change the code and it affects nothing outside of the class, to me that is low impact.

What does it become a design change? When it effect X object or X projects?

I’m just curious how others teams tackle this...

A: 

We refactor as often as we can. Having unit tests to ensure that everything works pre- and post- refactoring really helps.

rein
A: 

Code review processes often help with this. If I touch some code, it gets reviewed, reviewer asks, "why did you do it this way?", I say, "I had to because of (insert ugliness here)". This is a sign that the code should be refactored right after the review is done.

Jacob B
+2  A: 

Refactoring while you're already in the code is sometimes easiest, especially if your manager does not support the initiative, but if you only change a small part it will break consistency with surrounding parts. In these cases it's better to be selective and, as you suggested, do things that are low-impact. It may also be helpful to refactor long select/switch statements into functions and delay on refactoring the inner code until sometime later.

At a previous job, I was the manager, so I refactored whenever I wanted. At my current job, I'm an analyst so most of the code is not directly my responsibility. When I do write code, I avoid impacting anything that I'm not writing. I have one project which is entirely under my own control and I refactor any time I learn a better way to do something.

Scott
Finding support can be difficult, selling the clean-up to a non-developer manager or CEO is difficult. I have even had arguments with other developers because they are doing the code merge.
eschneider
+1  A: 

I work in a large system, so I only change things I have to. It is easy to have bad side effects to changes.

I will refactor sections of code that are performing poorly, not working properly, or needs new functionality.

I never just decide to fix things, I would never be done. if it works, and no one is asking for changes or complaining about problems, move on. life is too short to fix everything.

KM
+9  A: 
  • As part of original development (red/green/refactor)
  • When suggested by a code reviewer
  • When we've noticed a design pain-point
  • When making another change, if the refactoring is low impact, i.e. typically not affecting any other files.

If it affects the public API, I generally like to make the refactoring a single source code commit which doesn't change behaviour (and then build new behaviour into another commit). If it affects other projects too, there needs to be consensus over it and I would want to get permission to change their code to go in the same refactoring commit.

Jon Skeet
When the code is reviewed?
Daniel Moura
Before it's committed.
Jon Skeet
The whole code merge is a factor also, but I did not ask about that?
eschneider
"Design pain-point" IMO is more clearly expressed by "When adding a new feature is difficult, stop and refactor so that adding the feature becomes easy, then add it." :-)
xpmatteo
+1  A: 

I often refactor my code when there is a user requirement change or bug fixes. Then there will be a chance for people to review your changes.

Otherwise, I normally don't touch the workable code even it smells.

J.W.
A: 

To look at our company, we have decided that our upcoming application release is mostly dedicated to performance optimizations rather than new functionality. This was something we felt was needed and also was requested by some clients. Therefore we have spent a lot of time identifying performance bottlenecks in our app and reviewing code and refactoring it to make things run faster.

So in our case we did it because management approved us doing it for this new release, because we showed to them how much performance improvement could be gained.

tomlog
A: 

Refactor when needed:

  • when you need a better understanding of the code you are working on (pairing often helps here), examples are: renaming, method extraction etc.
  • when the current design doesn't allow for a 'clean' change: at this point you can actually argue with your manager on a value basis (e.g. what is this new feature worth to the project)
dimdm
A: 

I am always making small refactorings in my code. I know as long as I have my unit tests to verify that everything is still functioning properly afterward, I see no harm in doing it as I go. That way you don't get that vague "needs refactoring" feeling every time you work on it.

Now if it requires a large refactoring, it's best to plan for that and set aside some time.

ReadySquid
A: 

Seems most other posters are resistant to refacotring mercilessly. Of course this isn't possible if the system you're working on doesn't support this through extensive unit tests. But in general, If I can see an opportunity to make the code tighter without spending more than a few minutes or hours at most, I go for it. If I'm not sure what I should be working on, I look for something to refactor.

TokenMacGuy
+1  A: 

We found small refactorings are best done while we were working on a bit of code - do what's required, preferably paired.

For bigger things, we had a Technical Debt section on the wall - if you spotted something and didn't have the time to address it, or it was going to take some discussion to solve, you'd add it to the wall and they would be scheduled for future iterations (or when free time cropped up).

James
A: 

I refactor when I'm fixing a bug or adding a feature and the process of refactoring makes the code easier to read and easier to maintain.

plinth
A: 

Following DRY principles vehemently will often be a trigger for me to refactor.

Lance Harper
A: 

Insufficiently often, thus building up technical debt.

Sad, but so.

Do as I say, not as the team I work on does.

Jonathan Leffler
A: 

I find I refactor when revisiting code (presumably to add/extend functionality) more than 3 months after it was written.

If it takes me more than 2 minutes to discern what a chunk of code is doing, I'll break it apart to make it more immediately understandable (or just add some more comments.)

Crenshaw
A: 

as soon as all of the tests run.

Ray Tayek