2

When I'm doing large scale refactors I'm often commenting out the contents of methods and using NotImplementedExceptions for stuff that I still need to refactor. Problem is that this is interfering with 'valid' NotImplementedExceptions. I'm thinking about introducing a custom RefactorException, that I quite easily find back by reference to see what I still need to do.

Good idea or bad idea? What are other common ways to do large scale refactors in stages?

(of course the idea is that all these exceptions are removed before the I commit the whole refactoring)

Improve this question
6
  • related (possibly a duplicate): Pros and cons of custom exceptions (see also questions linked to it)
    – gnat
    Sep 5, 2014 at 9:12
  • Why not add a TODO comment? it's easy to grep for
    – ratchet freak
    Sep 5, 2014 at 9:20
  • If it is only for the developer to be aware why not using the message inside the exception. NotImplementedException("Refactor");
    – fantastik78
    Sep 5, 2014 at 9:31
  • Hi @ratchetfreak, my code is littered with TODO comments ;) usually they are low priority, but should be done at some point in time.
    – Dirk Boer
    Sep 5, 2014 at 9:53
  • @fantastik78, is also a possibility. Just thought it would carry more meaning using a custom Exception, plus you get the benefits of static typing in i.e. C# or Java.
    – Dirk Boer
    Sep 5, 2014 at 9:56

1 Answer 1

Reset to default
2

Refactoring is improving code in a way that doesn't affect it's functionality. That is, the public interface stays the same, but you might improve code formatting, algorithms, or implementation details. In a few cases, this might even be cleaning up a class hierarchy.

Because the behaviour of the code is the same before and after refactoring, you can use the same unit tests without change during refactoring. If you do not have tests, then writing tests for the part you are going to work on is the first step of refactoring:

  1. Write unit tests for the unit you're working on.
  2. Assert that the tests succeed with the current implementation.
  3. Refactor.
  4. Assert that the tests succeed with the new implementation.

You may have tests that don't test the externally visible behaviour, but test implementation details. Such tests make it easier to locate a problem, but they will have to be changed during step 3 “refactor”. The workflow is:

  1. Design the refactoring. I.e. think about what you're going to change, and how it should work afterwards.
  2. Rewrite the tests to test for the new implementation details.
  3. Assert that the tests fail with the current implementation.
  4. Refactor.
  5. Assert that the tests succeed with the new implementation.

If you have tests for implementation details, it is crucial to clearly separate these from tests that test the public interface.

This workflow guarantees that during a refactoring step you will not forget a required piece of functionality.

How would you mark regions of code that require refactoring? I recommend using a comment as TODO: REFACTOR because explanation. Such comments could be added during a code review. You can easily grep for such a comment in a code base. You suggested editing in a throw new RefactorException() – but that means …

  • … you can't use the software until the refactoring is performed. Shipping working code now can be better than shipping beautiful code tomorrow.
  • … you can't run tests while those exceptions are in the code.

Test-driven refactoring will also lead you to the areas that need improvement, but contrary to your exceptions will give you the certainty that the code behaves the same before and after.

Improve this answer
5
  • Hi amon, I'm talking about large scale code changes that change the class hierarchy. Formatting and changing the inside of some private methods I call 'cleaning up', and is not applicable to any of this.
    – Dirk Boer
    Sep 5, 2014 at 9:50
  • 2
    @DirkBoer changing the class hierarchy isn't refactoring but redesigning
    – ratchet freak
    Sep 5, 2014 at 9:55
  • Ahh I'm sorry. Guess I'm using the wrong terminology then. RedesignException? :)
    – Dirk Boer
    Sep 5, 2014 at 9:57
  • 1
    @DirkBoer If you are redesigning your code on a large and externally visible scale, then this is similar to adding a new feature. This involves writing tests and then writing code to satisfy these tests (assuming TDD). Writing the code may involve recycling code from the existing implementation. However, I'd advise against doing large-scale redesign by performing local changes on the existing structure. While getting tests to work you may need to create method stubs, but again the tests will remind you of filling them out, and you don't need a special NotImplementedException for that.
    – amon
    Sep 5, 2014 at 10:40
  • Hi @amon, I do see the value in automatic testing, but I do not practice TDD myself. For the moment I'm using Exception Driven Development (blog.codinghorror.com/exception-driven-development) and that works for me. Changing my whole developmentstyle for this particular question is a bit too much for now :) Thanks for your comprehensive answer!
    – Dirk Boer
    Sep 5, 2014 at 12:08

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

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