difference between expected inputs and implementation details

Question

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.

• max630: 8thlight.com/blog/uncle-bob/2014/05/10/WhenToMock.html
• Caleth: I don't Java enough to know what (unary) + does to Integers, does it return an unboxed int? The second is OK if it matches arithmetic operators, and flat wrong if it doesn't
• stijn: Passing SignedHelper to something as basic as abs looks like a textbook example of overengineering to me? Plus it could break abs if passing a SignedHelper which doesn't actually do what it's supposed to do, i.e. seems like breaking the Open/Closed principle. Likewise for 'arguments must reveal implementation detail', that's a strange one. I don't think I ever saw someone arguing for revealing implementation details. Is it possible your coworker is still learning and is going through a 'engineer all the things phase' instead of 'keep it simple'? I've been there once, it was a mess.
• Kevin: @max630 i think the "goldilocks" section of that article will be useful for him. thanks!
• Kevin: @Caleth i'm not sure. the main reason that abs(Integer) raises my eyebrow is that i believe (maybe incorrectly) that "int" is canonically used over "Integer" unless otherwise necessary. it would also open up the possibility of passing in null, which doesn't make sense. overall it would just make me wonder why a primitive type wouldn't work.

Answer 1

I'd say 'Math.abs(int someNumber, SignedHelper signedHelper)' is violating SRP(single responsibility principal). That is why it looks wrong.

You're passing an argument to the method and at the same time supplying a dependency the class needs.

Construction is a separate responsibility to ABS and the method is conflating the two. If the class needs SignedHelper as a dependency it should be injected into the constructor.

• Robert Harvey: There's nothing wrong with passing a helper object to a method. First-class functions do this all the time; e.g. sort pivots. It's just not a very convincing design in this particular scenario.

Answer 2

The signature of a method is the interface to the callers of the method.

In your case:

int Math.abs(int someNumber);

tells you that Math.abs() takes an integer number and returns another integer number. An important principle is that an interface should not change if the implementation changes.

In your context, if all the relevant implementations require a SignedHelper object, then it may be OK to pass it as a parameter. In this way you have a generic method Math.abs() that works with any implementation of SignedHelper.

On the other hand, if you can also have implementations of Math.abs() that do not use a SignedHelper at all, then you are correct that there should be no SignedHelper parameter: this is an implementation detail that should stay hidden inside the implementation and not leak to the interface.

What is the problem in the second scenario? Imagine you change the implementation to one that does not use a SignedHelper. Then

• either you need to change the interface and change all calls to the method throughout your code,
• or you keep the interface and ignore the argument in the implementation; the callers keep sending a dummy object just because it happens to be in the interface.

In the first case you are changing an interface: sometimes this is inevitable but changing an interface is a non-local change so it should be avoided if possible.

In the second case your interface does not describe the method properly. An API that requires dummy parameters is not well designed.

• Deduplicator: I would give the sole parameter a shorter name like x, because while due to technical limitations one has to give it a name, there's no info for it to convey.

Answer 3

These two signatures

Math.abs(int someNumber);

and

Math.abs(int someNumber, SignedHelper signedHelper);

Are somewhat comparable to the two versions of toString from the Integer class:

public static String toString(int i)

and

public static String toString(int i, int radix)

---

Both methods compute a result from an int and can do so without anything else given as a parameter. "Take the int and do your thing."

But maybe you want to get a string representation with a different radix. And maybe the internal implementation is capable of being variable in that regard, then why not expose that as another parameter?

That's exactly what happens in both cases. You have the option to modify how the internal implementation works.

Take the constructor of BufferedReader as another example:

public BufferedReader(Reader in, int sz)

WTF to me.

If you are not used to having the choice to specify the size of a "buffer" when working with a Reader, the additional parameter might come across as revealing information of the internal implementation - which is perfectly fine, because that's the whole point.

You have the option to pick a specific implementation if you need to and in that situation you definetly want those additional parameters. If you pick BufferedReader on purpose, you very likely want to specify the buffer size.

If you pick Math.abs(int someNumber, SignedHelper signedHelper) on purpose, you very likely want to specify what SignedHelper should be used.

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.

Whether its puzzling or not seems to be somewhat subjective. I often find overloads for methods in APIs that I cannot think of a use for. But somebody else might as well find them very useful.

Is there any harm in simply providing both versions of the method? Despite not being a requirement, is the version accepting the SignedHelper used anywhere in the code?

• Stop harming Monica: Yes, there is harm. If you implement interfaces that nobody will use you are wasting your time and somebody else's money.
• null: @Goyo hence the question if it is being used or not.
• Stop harming Monica: Well if the interface is being used then providing is a requirement.