I’ve probably seen this happen more in test code, where thinking is often even more procedural than normal. It’s certainly a test smell. However, I think this is a common anti-pattern in other code too.
01 02 03 04 05 06 07 08 09 10 11 12 13 14 | function readFile(filename) { // open file and return its string}function substituteId(file, id) { const placeholder = 'REPLACE-ME'; // do global replace of the placeholder with id // return that}// call siteconst file = readFile('foo.txt');const substituted = substituteId(file, currentId); |
The above is a perfectly fine bit of code. Maybe we want a general toolkit that allows us to open files and sometimes perform substitutions. However, if the two call usage of this appears more than a couple of times in the code, we may be in a situation where we’ve lost a level of abstraction.
What is the caller trying to do? Are they trying to open and file and play with it and then sometimes substitute things? Or are they trying to get hold of a processed file?
The example here is a common one for testing, where we’re using pre-created example data to represent the expected outcome of something where identifiers can change. The dilemma is that you either have to do fuzzy matching, or you have to doctor your expected to match the actual on the fields that vary each time.
However, the above two-step protocol is a common anti-pattern in general.
If you have to do the same two things every time, then you have two halves of a helper method… clearly that scales up to more obvious degrees of code duplication, but it’s easier to miss when it’s only two lines each time.
What’s the Solution?
If I said that we should write a third method to call the first two, you’d probably slap your head and say “Well, duh!” and you’d be right.
However, the solution has a nicer general case to consider.
Does the calling code express its intent, or get lost in the implementation?
With the above question, you can quickly spot when you’re having to do more work at the call site. Code should stay at a consistent level of abstraction. Test code, especially, should read like the intent of the test, not like the 75 moves you need to achieve that intent.
That said, over-refactoring tests can lead to not knowing what’s going on, but that’s another story.
Published on Java Code Geeks with permission by Ashley Frieze, partner at our JCG program. See the original article here: Half a Helper Method Opinions expressed by Java Code Geeks contributors are their own. |
