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?

  • 25
    Proper refactoring NEVER introduces breaking changes. See the definition, emphasis on "without changing its external behavior". – Carl Manaster Apr 5 '13 at 0:53
  • 10
    @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
  • 8
    @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
  • 8
    This sounds like a rant of a person with a bruised ego. – davidk01 Apr 5 '13 at 2:05
  • 3
    If your re-factoring broke regression you have done it wrong most of the time. – Rig Apr 5 '13 at 5:58

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 nowadays, 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


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 finds nothing. Realistically finding as little as possible.

  • 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

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.

  • 4
    "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

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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