Create instance of generic class member: Class.newInstance() throws exception

Question

I want to make a generic class which stores a pool of temporary objects (such as 2D vector Vec2d ) so that I don't have to dynamically allocate them during my simulations. Based on these answers [1] [2] I come to this solution:

import java.lang.reflect.Array;

class TempPool<T>{
  T[] objs;

  public TempPool(Class<T> c, int n) {
    T[] a = (T[])Array.newInstance(c, n);                // initialize the array
    try{ // Compiler forces me to use try fo handling                     
      for(int i=0; i<n; i++) a[i]=(T)c.newInstance();   // initialize fields in the array
    }catch(Exception e){
      System.out.println("Exception thrown  :" + e +" in TempPool()" );
    }
    objs = a;
  }

  T    borrow(  ){ return objs[icur++];    };
  void repay(T o){ 
    if( o != objs[icur] ) objs[-1]=null; // Throws exeption
    }else{ icur--; }
  }

But when I try to use

class Vec2d{ double x,y; }

tmpVec2d   = new TempPool       (Vec2d.class,10);   // tried both
//tmpVec2d = new TempPool<Vec2d>(Vec2d.class,10);   // tried both

it it throws exception Exception thrown :java.lang.InstantiationException: Boulders_rigid$Vec2d in TempPool()

Answer 1

Inner classes do not work how you think they work.

An inner class looks like this:

class Outer /* must be a class; not an interface */ {
    class Inner /* A class (not an interface), AND, not static {
    }
}

Inner class have a secret, invisible final field that looks like:

private final Outer magicThis;

that cannot be null. Every constructor that Inner has, has as first parameter Outer magicThis, but this parameter is also hidden. To pass it, you use this weird syntax:

outerInstance.new Inner();

and, crucially, which makes it 'magic', in the sense that you didn't know it worked this way, if Outer.this was a legal expression (which it is, in any non-static method inside Outer, for exmaple), Outer.this is silently assumed as outerInstance: In those contexts, and only in those, just new Inner() works.

Now that you know all this, you then understand that invoking newInstance() on a non-static inner class does not work: That requires a no-args constructor and there isn't one: All constructors have that secret, invisible Outer magicThis parameter.

You can observe all this by using the javap tool.

The solution is simple: Mark your inner class as static. In fact, go ahead and mark all your inner classes as static. Only make non-static inner class if you really know what you are doing and you are absolutely sure you really want that - the magic stuff tends to throw people off and make it easy to make wrong assumptions about code. For example, that secret ref to the outer? That can really trip you up with garbage collection (it prevents the outer instance from being collected!).

Note that your code has very crappy quality. A few things to look into:

1. The correct "I have no idea what to do" exception handler is NEVER to just log it and forget it, and System.out is not an appropriate logging locatino. Just e throws away almost all relevant information - in particular the cause which is what you do NOT want to throw away. The right I dunno handler for an exception block is always this: throw new RuntimeException("Unhandled", e);. This is short, simple, does not cause code to continue to run in an unknown state, and preserves all information. Fix this.

2. Don't call newInstance() on a class; it is deprecated. It has crazy exception behaviour. You want c.getConstructor().newInstance() instead.

3. objs[-1] = null of course throws ; there is no -1 index. Presumably you want objs[icur] = null? Or perhaps objs[objs.length - 1] = null;? I'm not sure what repay is trying to accomplish.

Answer 2

You shouldn't need to use reflection in this way.

Look at the source code of java.util.ArrayList: it doesn't declare an array of type E[] to hold the elements: it uses an Object[], and applies casts.

So, you can do this instead:

class TempPool<T>{
  Object[] objs;

  public TempPool(Class<T> c, int n) {
    objs = new Object[n];  // Simple!
    // ...
  }

  // Unchecked cast, but it's safe to suppress because you're ensuring only
  // instances of T are in here if you use it correct.
  @SuppressWarnings("unchecked");
  T    borrow(  ){ return (T) objs[icur++];    };
  
  // No changes needed to repay.
}

Also, instead of relying on reflection through Class<T>, pass a Supplier<T> (or, better, a Supplier<? extends T>): reflective instantiation is very error-prone, because you need to ensure that you're passing a Class with a zero-arg constructor that throws no checked exceptions etc (rzwitserloot's answer describes another of the problems specific to inner classes). A Supplier<T> is checked at compile time to ensure such a constructor exists; and it doesn't throw checked exceptions, so it substantially declutters the code.

  public TempPool(Supplier<? extends T> c, int n) {
    objs = new Object[n];  // Simple!
    for(int i=0; i<n; i++) objs[i]=c.get();
  }

Or, if you want to be streamy:

objs = IntStream.range(0, n).mapToObj(i -> c.get()).toArray();

You can now invoke like this:

tmpVec2d   = new TempPool<>(Vec2d::new,10);

---

Of course, a neater solution with no explicit casting would be simply to use a List<T>:

class TempPool<T>{
  List<T> objs;

  public TempPool(Class<T> c, int n) {
    objs = IntStream.range(0, n).mapToObj(i -> c.get()).collect(toList());
    // ...
  }

  // No cast required in borrow:
  T    borrow(  ){ return objs.get(icur++);    };