1

I wrote a Java API that accepts ClassLoaders as parameter, even though it won't accept any ClassLoader that isn't a URLClassLoader (because I'm only interested in the URLClassLoader::getURLs() method):

/**
 * @throws NullPointerException if classLoader is null.
 * @throws IllegalArgumentException if classLoader is not a URLClassLoader
 */
public void addClassLoader(ClassLoader classLoader) {
  if (!(classLoader instanceof URLClassLoader)) {
    if (classLoader == null) {
      throw new NullPointerException("classLoader must not be null");
    } else {
      throw new IllegalArgumentException("classLoader must be a URLClassLoader");
    }
  }
  this.classLoaders.add((URLClassLoader)classLoader);
}

I did this for several reasons:

  • adding a ClassLoader is a nice-to-have feature, not a must-have one: my API accepts several kinds of inputs;
  • the vast majority of ClassLoaders that users will want to give as parameters are indeed URLClassLoaders (from custom ones to web-app ones);
  • the ClassLoader isn't a very well known API and URLClassLoader is rather obscure and users shouldn't have to learn the ClassLoader capabilities in order to use my API;
  • casting a ClassLoader to URLClassLoader makes the client code less readable.

However I know that I should probably enforce the fact that I will only accept URLClassLoader in the method because it makes sense from my implementation details.

My question is: given all that is it OK to leave my method signature as addClassLoader(ClassLoader) and not addClassLoader(URLClassLoader)?

  • 2
    Why not go further and just take Object as your input, and throw on anything that isn't a URLClassLoader? I mean, no casting, ever, simplest API, vast majority of entities in client code are Objects. What can go wrong? – Ordous Jun 4 '18 at 12:55
  • If you care only for URLClassLoader::getURLS() why not requiring a collection of URLs as parameter ? It would reduce coupling inside your class and make (potential) tests easier to write. – Spotted Jun 5 '18 at 12:44
  • @Spotted I have this use case: "my API accepts several kinds of inputs" – Olivier Grégoire Jun 5 '18 at 13:21
  • Which kind of inputs ? – Spotted Jun 5 '18 at 13:38
  • URL, URI, Path,File, etc. (and their Iterable<X> version). Everything that is like a resource container in Java. – Olivier Grégoire Jun 5 '18 at 13:42
8

No, it's absolutely not OK.

A signature is a promise. You're breaking that promise. The fact that most of the time there will be no repercussions is a point against you, not for you: defects that cause problems rarely, unpredictably and in special situations are much worse than blatant ones. Blatant defects immediately cause problems and are rejected bu the community. Subtle, unlikely defects can make it all the way into production and into critical systems before being noticed.

TL;DR Don't go out of your way to create subtle defects.

4

I would enforce the URLClassLoader. If you accept a ClassLoader and you throw an exception.

My first thought was, that you hurt the Liskov substitution principle but it is the other way around: You don't accept the more general case.

But this is like beating the type system with your feet. Instead of letting your type system check your program while compile time you do it on runtime while you have no advantages.

I just created some tests (see below) to get the problem. It feels very unnatural that the third test is failing.

As an user, I would expect, that the type system should catch the third (failing) test case. In fact I just started the debugger to dig in to reason about.

This should not the case: The interface should should express the intention of the method and if you look at the the methods body, it should be the same as you expected to see. This is not not the case here.

@Test
void shouldTakeUrlClassLoader() {
    ClassLoaderExample uut = new ClassLoaderExample();
    uut.addClassLoader(new URLClassLoader(new URL[]{}));
}

@Test
void shouldTakeSubtypeOfUrlClassLoader() {
    ClassLoaderExample uut = new ClassLoaderExample();
    uut.addClassLoader(new SpecializedLoader(new URL[]{}));
}

@Test
void shouldTakeClassLoaderAsArgument() {
    ClassLoaderExample uut = new ClassLoaderExample();
    uut.addClassLoader(new ClassLoaderStub());
}

class ClassLoaderStub extends ClassLoader {}

class SpecializedLoader extends URLClassLoader {
    SpecializedLoader(URL[] urls) {
        super(urls);
    }
}

[EDIT]. The third case fails with java.lang.IllegalArgumentException: classLoader must be a URLClassLoader

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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