How to remove a function or feature when using TDD

Removing a public method is not "refactoring" -- refactoring is changing the implementation while continuing to pass existing tests.

However, removing an unneeded method is a perfectly reasonable design change.

TDD draws this out to some extent, because in reviewing the tests, you may observe that it's testing an unneeded method. The tests are driving your design, because you can go "Look, this test has nothing to do with my goal".

It may reveal itself more at higher levels of testing, in conjunction with code coverage tools. If you run integration tests with code coverage, and see that methods aren't being called, it's a clue that a method isn't used. Static code analysis can also indicate that methods aren't used.

There are two approaches to removing a method; both work in different circumstances:

1. Delete the method. Follow the compilation errors, to delete any dependent code and tests. If you're satisfied that the affected tests are disposable, commit your changes. If not, roll back.

2. Delete the tests you feel are obsolete. Run your entire test suite with code coverage. Delete methods that haven't been exercised by the test suite.

(This presupposes that your test suite has good coverage to start with).

In fact f() replaces all calls to b() with the exception of the unit tests that defined / described b()

IMHO the typical TDD cycle will look like this:

• write failing tests for f() (probably based on the tests for b()): tests go red

• implement f() -> tests become green

• refactor: -> remove b() and all tests for b()

For the last step, you might consider to remove b() first and see what happens (when using a compiled language, the compiler should complain only about the existing tests, when not, the old unit tests for b will fail, so it is clear you have to remove them, too).

Yes, it is.

The best, most bug-free, most readable code is the code that doesn't exist. Strive to write as much non-code as possible while fulfilling your requirements.

It's desirable to remove b() once it's no longer used, for the same reason that it's desirable not to add un-used functions in the first place. Whether you call it "readability" or something else, all else being equal it's an improvement to the code that it doesn't contain anything it has no use for. For the sake of having at least one specific measure by which it is better not to have it, removing it guarantees that its future maintenance cost after that change is zero!

I haven't found any special technique to be needed for actually removing it with its tests, since any thought of replacing b() with something new must of course be accompanied by a consideration of all code currently calling b(), and tests are a subset of "all code".

The line of reasoning that generally works for me is that at the point where I notice that f() has made b() obsolete, therefore b() should be at least deprecated, and I'm looking to find all calls to b() with the intention of replacing them with calls to f(), I consider also the test code. Specifically, if b() is no longer required then I can and should remove its unit tests.

You're quite correct that nothing forces me to notice that b() is no longer required. That's a matter of skill (and, as slim says, code coverage reports on higher-level tests). If only unit tests, and no functional tests, refer to b(), then I can be cautiously optimistic that it's not part of any published interface and therefore removing it is not a breaking change for any code not under my direct control.

The red/green/refactor cycle doesn't explicitly mention removing tests. Furthermore, removing b() violates the open/closed principle since clearly your component is open for modification. So if you want to think of this step as something outside simple TDD, go ahead. For example, you might have some process for declaring a test "bad", which can be applied in this case to remove the test on the grounds that it tests for something that shouldn't be there (the unnecessary function b()).

I think in practice most people probably allow for a certain amount of redesign to be carried out along with a red/green/refactor cycle, or they consider removing redundant unit tests to be a valid part of a "refactor" even though strictly-speaking it is not refactoring. Your team can decide how much drama and paperwork should be involved in justifying this decision.

Anyway, if b() was important then there'd functional tests for it, and those would not be removed lightly, but you've already said there's only unit tests. If you don't properly distinguish between unit tests (written to the code's current internal design, which you have changed) and functional tests (written to published interfaces, which perhaps you don't want to change) then you need to be more cautious about removing unit tests.

One thing to always try to remember is that we're now using CODE REPOSITORIES with VERSION CONTROL. That deleted code isn't actually gone...it's still there somewhere in a previous iteration. So blow it away! Be liberal with the delete key, because you can always go back and retrieve that precious elegant method that you thought might be useful someday...if that someday ever comes. It's there.

Of course, that goes along with the caveat of the ills and danger of non-backwards-compatible releases...external applications that relied on your interface implementation, that are now orphaned by your (suddenly) deprecated code.