Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

Recent QA testing has found some regression bugs in our code. My team lead blames recent refactoring efforts for the regressions.

My team lead's stance is "refactor, but don't break too many things", but wouldn't tell me how many is "too many". My stance is it's QA's job to find bugs that we can't find, and refactoring usually introduces breaking changes. So sure, I can be careful, but I don't knowingly release code with bugs to QA. I do it because I don't see them.

If the refactoring was necessary, how many regression bugs should be considered too many?

share|improve this question
16  
Proper refactoring NEVER introduces breaking changes. See the definition, emphasis on "without changing its external behavior". –  Carl Manaster Apr 5 '13 at 0:53
7  
@Carl: In theory, yes. In practice, doing so is equivalent to "writing code with no bugs," and we all know how often that happens. In fact, it seems to me that if you truly understood what might be affected by changing the code in question, you wouldn't need to refactor it in the first place. –  Mason Wheeler Apr 5 '13 at 0:58
2  
Your code is your house. How many bugs would you tolerate and at what level of visibility? I'd be okay with a couple of moths over a lurking black widow. –  World Engineer Apr 5 '13 at 1:01
6  
@MasonWheeler: "Proper" Refactoring involves tiny steps that prevent such errors. It also involves a test suite for added protection. Anything else is reengineering, which also supposedly doesn't change the underlying functionality, but has a name that scares the life out of battle-worn managers. Refactoring is starting to have the same effect, because we've been promising that it won't introduce changes, and then introducing bugs, for long enough now. But it's ok, we'll just think of a new safe-sounding name, right? –  pdr Apr 5 '13 at 1:12
6  
This sounds like a rant of a person with a bruised ego. –  davidk01 Apr 5 '13 at 2:05

2 Answers 2

up vote 16 down vote accepted

You are right that refactoring code is important. It prevents code rot and improves code. It makes for cleaner code.

But good code is not only clean code, it's code that is correct, and thus by definition contains as few bugs as possible (ideally none). The first goal of your code is to produce its expected result. So if your refactoring is introducing bugs you might want to consider the net effect of those refactorings.

You should refactor code that is tested. If it's not, add tests and then refactor. This way you know you haven't broken anything. This will help mitigate the risks of a similar situation from happening in the future.

As for refactoring introducing bugs, refactoring should not alter the behavior of a program. I will quote from Wikipedia:

"disciplined technique for restructuring an existing body of code, altering its internal structure without changing its external behavior"

Sadly now days refactoring has come to mean anything from this definition to total rewrites.

I would alter what your team lead said from:

"refactor, but don't break too many things"

To:

"refactor and don't break anything"

As for finding bugs being the job of the QA, quality isn't someone else's problem. The goal should be that QA find's nothing. Realistically finding as little as possible.

share|improve this answer
1  
Actually, QA's job is to ensure the Quality processes have been followed. The act of testing is QC. If QA are finding bugs, you have a serious problem –  mattnz Apr 5 '13 at 6:50
    
+1; these are all excellent points. –  Carl Manaster Apr 5 '13 at 14:14

My stance is it's QA's job to find bugs that we can't find,

Finding bugs is a joint responsibility.

QA's job is to find bugs that you don't find. There should not be such a thing as bugs that you can't find. (And if there is, then maybe you should ask for access to the tools / tests that they are using so that you can find them for yourself.

refactoring usually introduces breaking changes.

That is not true. Refactoring done properly doesn't break things. And you could argue that fixing the things that break should be part of the refactoring.

The problems arise in reality when:

  • the refactoring triggers latent bugs in other parts of the codebase that the tests you can run don't reveal, or

  • you can't do the full refactor because someone else "owns" some parts of the code that is affected by the refactor.

You shouldn't cop the blame for latent bugs (unless they were yours in the first place). But on the other hand, if your refactoring is intentionally changing APIs and or behaviours then you need to discuss this with all parties before you refactor, and have a plan for dealing with the effects. On the third hand1, if the breaking change is unexpected or accidental ... then you should have thought it through in more detail.

And finally, if the regressions were your fault (or partly your fault), then "man up" and take the blame. And aim to do better next time.

If he agrees that refactoring is necessary (which he does), then why does he have a problem when stuff breaks? Especially when it hasn't affected our schedule so far.

That is easy to answer if you put yourself into his shoes. He thinks you did the wrong thing in the way you refactored the code. This time it didn't impact on the schedule. But next time, it could. He is trying to prevent that eventuality.


1 - A Larry Niven allusion.

share|improve this answer
2  
"if your refactoring is intentionally changing APIs and or behaviours..." -- then it's not a refactoring anymore? "disciplined technique for restructuring an existing body of code, altering its internal structure without changing its external behavior" –  gnat Apr 5 '13 at 16:29
1  
@gnat - The Question is about "refactoring" in the sense that the OP is using that term, and so is my Answer. It is patently clear that his refactoring did break something. Whether you call it an "incorrect refactoring" or "not a refactoring at all" is semantic hair-splitting ... and not helpful. –  Stephen C Apr 6 '13 at 5:27
    
@gnat - And besides, whether an API or behaviour is external or not depends on the scope of your refactoring activity ... –  Stephen C Apr 6 '13 at 5:30
    
now that you explained what you mean, I understand (and tend to agree). However, for those who read that phrase in your answer it really looks like your belief simply contradicts the standard definition –  gnat Apr 6 '13 at 6:13
    
@gnat - I am using the term in the way it is commonly used. A definition should reflect common usage, not the other way around. There is no such thing as a "standard" definition, because there is no body that is officially tasked with standardizing IT definitions. Wikipedia definitions are useful ... but they are not "the standard". –  Stephen C Apr 6 '13 at 13:13

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.