4

Given a class that implements an interface, but does not need all of the methods implemented, what is the best practice for unit testing this class with respect to code coverage? - or is it considered a sign for a code smell?

To make the problem more concrete, consider a Java class (the question is not limited to Java though) that implements ComponentListener but derives from some X (so as to rule out the choice to use ComponentAdapter). The class is however only interested in the componentResized() method and leaves the other callback method bodies empty.

When checking the code coverage, we get a report that indicates correctly that the remaining methods are untested. For obvious reasons, I hesitate to add a test that simply calls a no-op method for the sake of coverage.

While I am not bound to reach a certain coverage, I still think that in and of itself, this phenomenon may indicate a code smell with respect to the single-responsibility principle. On the other hand, it's not far-fetched either that a component representation is responsible for updating its state on a resize.

Is there some sort of consensus or best practice on how to handle such methods, or is the question illegitimate as in it is a result of a supposedly bad design?

4
  • Instead of single-responsibility principle, perhaps you mean interface segregation principle?
    – rwong
    Commented Sep 11, 2013 at 10:34
  • Frank: perhaps you can give a concrete example of such listener interface, preferably taken from Core Java or some well-known library, so as to: (1) illustrate a situation where it makes sense to selectively listen to only a few of the listener methods, and (2) to explain that this situation is common enough that a solution is needed. (That is, programmers are faced with a design decision made by library vendors, and therefore programmers need to find a solution to this problem, even if it falls into the "bad design" category.)
    – rwong
    Commented Sep 11, 2013 at 11:14
  • The example given is actually the one I encountered: a component listener, where the component didn't care about being shown/moved/etc., but about being resized (as that affected f.ex. the drawing code)
    – Frank
    Commented Sep 11, 2013 at 17:43
  • @rwong: yes, I guess that applies just as well in those cases.
    – Frank
    Commented Sep 11, 2013 at 17:44

5 Answers 5

6

I'm not sure what the best practice would be but to me, your problem isn't a problem of empty interface methods - it's a problem that your interfaces are wrong.

If you implement an interface, implement the entire interface - if the interface is too broad or has functionality that you're not interested in then split the interface and have the main interface implement sub-interfaces and have your component only implement the bit that's appropriate.

4
  • 1
    True indeed. I count this answer as a bad design answer then, though the interface being a 3rd party one is not available for re-design.
    – Frank
    Commented Sep 11, 2013 at 9:29
  • 1
    +1 I agree with Michael. 'Interface' means 'contract' so you definitely should implement the whole interface.
    – pepuch
    Commented Sep 11, 2013 at 9:32
  • Does the implementing class definitely behave correctly without implementing the functionality? There must be code out there which depends on this code being implemented or assumptions made elsewhere that your implementing class will do something as a consequence of the callback?
    – Michael
    Commented Sep 11, 2013 at 9:43
  • 1
    What about interfaces such as javax.servlet.Filter? In many filter implementation, there's nothing that you would want to do in the 'init' and 'destroy' methods. I guess the question is still valid. Commented May 25, 2015 at 7:22
2

An Interface represents a contract that your implementation class agrees to fulfill. The parts of that contract that you are not interested in, you are still implementing and therefore they still have behavior - the behavior being either do nothing (as I expect in your example) or throwing an UnsupportedOperationException if you are purposefully reducing the scope of the implementation (such as List#Clear).

I think it is still important to a) document the lack of implementation, b) add tests to validate the implementation so that when someone comes along and changes it, your tests break and they have to think a little before modifying your IOnlyCareAboutResizedComponentsListener class.

1

I would write a test for the interface, which calls all methods and ignores the results (just to check if no exceptions are thrown ant any method). Than you can make your test-class extend this test-class of the interface.

This way you get full coverage.

2
  • I know that just calling the methods raises coverage, but it is a thorough waste of time for the sole sake of increasing a coverage number.. that just doesn't sound like the right way.
    – Frank
    Commented Sep 11, 2013 at 17:45
  • 1
    @Frank: just wait until a new developer (or architect!) joins the team and start putting throw new NotImplementedHereException(); into these empty function bodies. It's just a minor refactoring.
    – rwong
    Commented Sep 11, 2013 at 19:32
1

Michael's answer shares my feeling that it is a design smell in the first place.

If it is an empty callback that gets called at runtime, you should write the test to make sure that nothing happens in that method.

If the method is not implemented and must never be called, you should consider throwing a java.lang.UnsupportedOperationException.

-1

I think a better pattern if you need code coverage is using an abstract factory. The difference between interfaces and abstract classes already has had some discussion on this site. I find this explanation the most relevant for your question.

Are abstract classes / methods obsolete?

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.