Throw exception in interface default method

Question

Recently I came across this code.

public interface CustomerQueryService {    
    default Customer getCustomerById(long id) {
        throw new NotImplementedException();
    }
}

Later, it turned out that it's general convention for this project. Is it considered a good practice?

Ultramontanism

So, while the usage you outline is often acceptable (it's the "optional method" idiom), the fact that this is a "general convention" in your project is a huge screaming red flag. To put this in perspective, I've used this pattern only a handful of times (for example, Iterator.remove().) To see it used regularly as a "general convention" is almost certainly overuse. – Zollverein

More generally, regular use of such a convention is effectively downgrading "implements interface" from a statically typed property to a dynamically typed one. Which will invariably blow up in the hands of unsuspecting and undeserving users, since they have reason to believe that "implements Foo" really means "implements Foo." True optionality is rare; treat it as such. – Zollverein

@BrianGoetz And for the record; I feel very much honored in your presence ;-) – Psycholinguistics

Answer 1

One could starte here to see what Brian Goetz, one of the "acting fathers" of Java has to say about "how to use default methods".

So, according "to the books"; one could use them like in your example: to throw an exception for a method regarded "optional". But that would mean: you have already some methods; and you are adding new ones.

The main purpose of adding default methods to the Java language was to allow for non-breaking enhancements of existing interfaces. They were not meant to be used as some sort of "mixin" / multi-inheritance / traits-like providing construct.

But beyond that: in your example, you have a new interface ... that only has that one method. I think this does not fall under those "intended usages".

On the other hand; don't be too much "about the books". When a whole team of people agrees "this is what we do", and everybody understands and buys into that; why not?!

But there is one caveat ... my C++ coworkers have a strict policy: they allow for exactly one implementation of any abstract method within an inheritance tree; as it is very hard to debug a problem when you are looking at the wrong implementation of some method. And now that we can inherit default methods in Java, too ... debugging problems could become harder in Java for us in the same way. So be careful how such things are used!

Long story short: it is a good practice if the big majority of your development team finds it a helpful practice. If not, it is not.

Psycholinguistics

I agree, but I'd suggest improving by quoting sources where applicable. – Holocaine

Answer 2

I'd say it's bad because of the exception type and the reports that it's a convention in the project.

NotImplementedException:

NotImplementedException represents the case where the author has yet to implement the logic at this point in the program. This can act as an exception based TODO tag.

So this is a lazy way to provide an implementation for a method across all CustomerQueryService just to find out at runtime that you've yet to write it.

Note UnsupportedOperationException might be an acceptable one in some cases, but neither would be a good "convention" for a project.

Steerageway

@Loc OK, was just a coincidence that downvote was at the exact same time as your reply on your answer. – Steerageway