Sunday, 5 November 2006

Generics == bashing your head against a wall

Well, I've been spending a few hours generifying commons-collections recently. The jobs only just started, and my head hurts from all the times I've banged it against the wall. Is there anybody listening who might actually be able to communicate to the powers that be in Sun that generics are just way messed up?

This time, my problem is in CompositeCollection. It holds a collection of collections, and makes it look like a single collection. Simple enough use case. So here is a simple generification (showing the methods that add a composited collection to the collection of composited collections):

public class CompositeCollection<E> implements Collection<E> {

  private Collection<Collection<E>> all;

  public void addComposited(Collection<E>);
  public void addComposited(Collection<E>, Collection<E>);
  public void addComposited(Collection<E>[]);

}

Thats great! It works and we're all happy, right? Well, not really, because those three addComposited() methods can be simplified using varargs of course:

public class CompositeCollection<E> implements Collection<E> {

  private Collection<Collection<E>> all;

  public void addComposited(Collection<E>...);

}

Now isn't that a much simpler API. We can pass in an array, one, two, or any number of collections we like, right? Wrong.

The problem is that calling this method is totally screwed due to the messed up generics implementation:

public class MyClass {

  private void process() {
    Collection<String> a = new ArrayList<String>()
    Collection<String> b = new ArrayList<String>()
    CompositeCollection<String> c = new CompositeCollection<String>;

    c.addComposited(a, b);    // compiler warning!!!!!!!!
  }

}

Why is there a compiler warning? Because in order to pass a and b to the method the varargs implementation needs to create an array - Collection<String>[]. But the generics implementation can't create a generified array, yet thats what the method is demanding. Arrrgh!!

So, instead of being helpful, Java creates an ungenerified Collection[], and then the generics system warns us that it is an unchecked cast to make it a Collection<String>[]. But this conversion is all internal to the compiler! As far as the developer is concerned, there is nothing even vaguely type unsafe about what s/he is doing.

Now this is just headbangingly stupid. There is absolutely no risk of breaking the generics model here. No risk of a ClassCastException or ArrayStoreException. No, this is really just another case of generics being nearly completely broken with regards to arrays. And the varargs group didn't manage to realise/coordinate and get a decent implementation that works.

The problem for me is serious though. I am writing an API (commons-collections) that will be widely used, and will have to retain backwards binary compatibility. If I go with the varargs option (the logically correct choice), then every client of this method will get a useless java warning - not very friendly. The alternative is to go back to the original design and have three methods, an array, one-arg and two arg. And really thats just crap.

17 comments:

  1. It's type unsafe becuase you can assign elements of the generic array without warning.

    I think the problem is with varargs rather than generics.

    Is there a particular need for this method anyway. Can't you just go for a sequence of simpler methods?

    ReplyDelete
  2. I'm glad I stopped doing Java before generics came out.

    ReplyDelete
  3. Some people has already done it:
    http://larvalabs.com/collections/

    slava: i'm glad you stopped doing Java. i would be glad if you stop talking in Java related blogs too.

    ReplyDelete
  4. afsina: I would, but its like watching a train wreck, can't stop. Schadenfreude, if you will.

    ReplyDelete
  5. Stephen Colebourne6 November 2006 at 12:44

    Tom - Its not type unsafe to the developer. The type unsafe array is created and disappears within the addComposited(a, b) method call.

    As far as the developer is concerned, they did nothing wrong - they passed in two Collection to a method which expected a Collection... which would treat it as a Collection[]. No developer issue makes this almost a compiler bug.

    afsina - I'm fully aware of the larvalabs port, and thats great. The new commons-collections version has a different goal however, in that it will try and make a better API suited to JDK5, rather than a port of a JDK2 designed API.

    ReplyDelete
  6. Slava, did you have any problems as a child?

    ReplyDelete
  7. Remember that varargs use arrays underneath, and that arrays are statically not typesafe in Java. That's why they are not compatible with generics.

    Simple example:

    Number[] numbers=new Double[10];
    numbers[1]=5; //autoboxed int to Integer there.

    This will compile but give an ArrayStoreException at runtime. Arrays need the 'type token', and generics don't supply it. The usual solution is not to use an array. Unfortunately we don't get a choice with varargs, so the solution is not to use varargs.

    In your example you could pass a Collection> instead of a 'Collection...'

    If the compiler let you 'get away' with this, you could still make type errors, because the vararg parameter is actually an array, despite the '...' syntax - it can come directly from an array when it's called, etc.

    I'd prefer it if varargs boxed to some implementation of Iterable than to an array.

    ReplyDelete
  8. I had a similar problem with some jax-ws stuff, it ended up being classified as a JDK (Mustang/1.6) problem. Just out of interest, my problem only occurred when compiling with debug turned ON. If you turn debug off does the compiler warning go away ?

    Heres the relevant JDK bug:
    http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6468404

    Rob

    ReplyDelete
  9. Osvaldo Pinali Doederlein6 November 2006 at 22:05

    Stephen: It seems that you can't do that varargs simplification, because the resulting class would only contain one method, which erased signature would be an array: addComposited(Collection[]). But this signature is not binary-compatible with legacy client code that makes invocations to the addComposited(Collection) and addComposited(Collection,Collection). Legacy code would only run if recompiled from sources, and with -source 1.5. So if you care about backwards binary compatibility, you gotta keep the three methods anyway...

    ReplyDelete
  10. Stephen Colebourne6 November 2006 at 22:12

    Osvaldo: For this version of commons-collections it won't be backwards compatible, either source or binary. As a result it will be ending up in a new package, allowing gentle migration to the generified implementations. This may prove a little controversial to some, but we believe that it will produce a better, more Java5 style, API.

    ReplyDelete
  11. http://sourceforge.net/projects/collections

    ReplyDelete
  12. Some stuff

    http://www.mernst.org/blog/archives/11-01-2006_11-30-2006.html#132

    ReplyDelete
  13. My suggestion: turn the method into a chainable method:

    public CompositeCollection add(Collectionc);

    this way, you can chain all the collections:

    composite = new CompositeCollection()
    .add(a).add(b);

    ReplyDelete
  14. Stephen Colebourne7 November 2006 at 21:46

    Tiago: I agree that making it chainable might solve some of the pain. However it doesn't help if you already have an array or collection of collections. I guess the point of this blog is that there is no simple API answer to this kind of problem.

    ReplyDelete
  15. If you already have a collection of collections, then you don't have a problem, you can use a method that accepts Collection> (or Iterable>, more likely).

    If you have an array of collections, you either aren't using generics or you don't have compilable code. The only way I can create a generic array is with a warning, via varargs. Directly it doesn't work (compile error).

    If it's neither of these cases, the chainable approach would work.

    Maybe it's also worth considering whether CompositeCollection actually needs to be ported to 1.5 - did anyone use it much? You could leave it out and wait for the complaints. ;)

    I don't see anyone using CompositeCollection if I search for it in Google Code Search or koders.com

    I certainly can't come up with a compelling case where I think I'd use it. Emulating a 2D array? I'd rather use a 1D list or a Map,Whatever>.

    ReplyDelete
  16. IMHO Sun messed up big time with object arrays and generic collections. Object arrays, should be treated as instances of RandomAccess lists (via compiler magic), and both object arrays and Generic collections should have an attached filter type, for runtime protection. I really think it is utter stupidity that Sun did not do generics properly, given backward compatibility could easily have been supported by defaulting the filter type of legacy data to unfiltered.
    I think that the arguments about runtime type checking performance issues are nonsense given that the Java class loader should be able to make proper typed variants of classes on-the-fly. To help this to work, Sun would have to stop being stupid and accept that typedefs are critical and not just a C hangover.

    ReplyDelete