During a code review, I suggested to my coworker that he had questionable parameters for one of his methods. In particular, the parameters revealed too much about the method's implementation details. My coworker argued that a method's inputs must reveal the method's implementation details, so, where does one draw the line? I could kinda see what he was saying, but I struggled to articulate the distinction between "inputs" vs "implementation details".
Contrived examples (he would argue all of these are OK):
- int Math.abs(int someNumber); // ok to me
- int Math.abs(Integer someNumber); // slightly interesting to me
- int Math.abs(int someNumber, SignedHelper signedHelper); // SignedHelper is a class that tells you whether numbers are signed or unsigned. WTF to me.
Things I've said that are unsatisfactory to him:
- You would expect Math.abs() to take a primitive type. Why should it be a capital-i Integer?
- The presence of SignedHelper reveals too much about how Math.abs() works.
- You would expect Math.abs() to figure out what a signed number is - the user shouldn't need to care. That's an implementation detail.
- The presence of SignedHelper implies that it's possible to pass an implementation of SignedHelper that behaves differently than what you'd expect (which could be a possible feature, I guess, but in our case wasn't an ask or expectation at all). This is puzzling and raises questions about the motives of this method.
- If we pretend that Math.abs() actually DOES use SignedHelper in its implementation, it's better to test SignedHelper separately than to expose it in the parameters to enable injection of a mocked SignedHelper. (this was in response to one of his concerns, where he wanted to test only the code that used SignedHelper, rather than testing any of SignedHelper along with the code in question)
So, how would you explain the difference between inputs that make sense vs inputs that don't make sense? I sense he is being argumentative but at the same time I'd like to be able to articulate something that is just obvious to me, because it's quite possible I actually don't have a good understanding.