New Refactoring: Remove Feature

If you’re using a modern, full-featured IDE like IntelliJ or Rider, you may have used an automated refactoring called Safe Delete. This is a jolly handy thing that I use often. If, say, I want to delete a Java class file, it will search for any references to that class first. If the class is still be used, then it will warn me.

Occasionally, I want to delete a whole feature, though. And for this, I am imagining a new refactoring which I’m calling – wait for it! – Remove Feature. Say what you see.

Let’s say I want to delete a public method in a class, like rentFor() in this Video class.

Like Safe Delete, first we would look for any references to rentFor() in the rest of the code. If there are no references, it will not only delete rentFor(),  but also any other features of this and other classes that rentFor() uses – but only if they’re not being used anywhere else. It’s a recursive Safe Delete.

isUnderAge() is only used by rentFor(), so that would be deleted. CustomerUnderageException is also only used by rentFor(), so that too would be deleted. And finally, the addRental() method of the Customer class is only used here, so that would also go.

This is a recursive refactoring, so we’d also look inside isUnderAge(), CustomerUnderageException and addRental() to see if there’s anything they’re using in our code that can be Safe Deleted.

In addRental(), for example:

…we see that it uses a rentals collection field. This field is also used by getRentedVideos(), so it can’t be Safe Deleted. Our recursion would end here.

If the method or function we’re removing is the only public (or exported) feature in that class or module, then the module would be deleted also. (Since private/non-exported features can’t be used anywhere else).

But if customers can’t rent videos, what’s the purpose of this field? Examining my tests reveals that the getter only exists to test rentFor().

One could argue that the feature isn’t defined by the API, but by the code that uses the API – in this case, the test code reflects the set of public methods that make up this feature.

So I might choose to Remove Feature by selecting one or more tests, identifying what they use from the public API, and recursively Safe Deleting each method before deleting those tests.

Author: codemanship

Founder of Codemanship Ltd and code craft coach and trainer

One thought on “New Refactoring: Remove Feature”

  1. That seems like a good idea.
    Maybe another perspective on it would be helpful:
    How about a “Delete dead and commented-out code.” refactoring?
    That would be a true behavior-preserving refactoring. (… if done correctly.)
    (Clearly, provisions would have to be made for (1) reflection usage and (2) usage from outside the code base you’re looking at.)
    And if something is ONLY used from the xUnit tests, then that’s suspicious: Maybe it should be deleted.

    And …
    If you comment out the (hopefully) one and only one line that “exposes” the feature for use (and ties it to the code that implements it), and then ran the “Delete dead and commented-out code.” refactoring, … I think you’d get the “Remove Feature” refactoring you describe above.

Leave a Reply

Fill in your details below or click an icon to log in: Logo

You are commenting using your account. Log Out /  Change )

Google photo

You are commenting using your Google account. Log Out /  Change )

Twitter picture

You are commenting using your Twitter account. Log Out /  Change )

Facebook photo

You are commenting using your Facebook account. Log Out /  Change )

Connecting to %s