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.
public void rentFor(Customer customer) throws CustomerUnderageException { | |
if(isUnderAge(customer)) | |
throw new CustomerUnderageException(); | |
customer.addRental(this); | |
} |
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:
public List<Video> getRentedVideos() { | |
return rentals; | |
} | |
public void addRental(Video video) { | |
rentals.add(video); | |
} |
…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().
@Test(expected=CustomerUnderageException.class) | |
public void customerMustBeOverTwelveToRentAVideoRatedTwelve() throws Exception { | |
Customer customer = new Customer(null, null, "2012-01-01"); | |
Video video = new Video(null, Rating.TWELVE); | |
video.rentFor(customer); | |
} | |
@Test | |
public void videoRentedByCustomerOfLegalAgeIsAddedToCustomersRentedVideos() throws Exception { | |
Customer customer = new Customer(null, null, "1964-01-01"); | |
Video video = new Video(null, Rating.TWELVE); | |
video.rentFor(customer); | |
assertTrue(customer.getRentedVideos().contains(video)); | |
} |
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.