Integer.class and int.class as Guice Keys

Shortly after fixing arrays, I've found another multiple representations bug. This problem is probably familiar - I'm confusing primitive types (like int) with wrapper types (like Integer).

It's one binding

The critical question: should these tests pass?

    assertEquals(Key.get(int.class), Key.get(Integer.class));
    assertEquals(TypeLiteral.get(int.class), TypeLiteral.get(Integer.class));

Currently these are non-equal, so Guice has special cases so that they both work. But some special cases are missing! Consider issue 116:

    Injector injector = Guice.createInjector(new AbstractModule() {
      protected void configure() {
        bind(int.class).toInstance(1984);
      }
    });
    assertEquals(1984, (int) injector.getInstance(int.class)); /* passes */
    assertEquals(1984, (int) injector.getInstance(Integer.class)); /* passes */
    assertEquals(1984, (int) injector.getInstance(Key.get(int.class)));  /* passes */
    assertEquals(1984, (int) injector.getInstance(Key.get(Integer.class)));  /* passes */
    assertNotNull(injector.getBinding(Key.get(int.class)));  /* passes */
    assertNotNull(injector.getBinding(Key.get(Integer.class))); /* fails! */

Should Key be fixed?

Yes. I think I'll change Key so that it always uses Integer.class, regardless of whether it was created with int.class or Integer.class. Otherwise, this stuff is just too prone to bugs. For example, our new Binder SPI can return Provider<int>, even though that's not a valid type.

Should TypeLiteral be fixed?

Probably not. I'm leaning towards leaving it as-is. Consider an interface with these methods:

    public boolean remove(Integer value)
    public Integer remove(int index);

If I change TypeLiteral, then the "remove" method is ambiguous when I know the TypeLiteral of the parameter type. But as a side effect of being inconsistent with Key, this test will always fail:

  TypeLiteral<Integer> primitive = TypeLiteral.get(int.class);
  TypeLiteral<Integer> wrapper = new TypeLiteral<Integer>() {};
  assertEquals(Key.get(primitive), Key.get(wrapper));
  assertEquals(primitive, wrapper);

I think it's a fair compromise.

Fixing the right thing

By making my changes to Key and TypeLiteral, I can make Guice behave consistently throughout all of its APIs. I won't have to worry about users who bind both int.class and Integer.class. And I should be able to rip out some special cases both Guice and its extensions.

If there's a Guice issue that's you'd like fixed, this is a great time to get your feelings heard. I'm spending a lot of time on the issues list, trying to decide what will make the cut for 2.0.

# posted by Jesse Wilson on Sunday, June 01, 2008