flatbuffers: Java: public default constructor is error-prone

Hi!

I am currently using flatbuffers to both hold default values and allow to sync user themes (for an Android Wear Watch Face) between the handset and the wearables. I always need a Themeobject (which is a flatbuffers Table) available to get the default values if the user hasn’t set any theme. How to create an empty object is not clear.

From the code (and I tried to confirm), creating a new object initially (new Theme()) is possible, and creates one with a null``ByteBuffer, leading to a NullPointerException as soon as you call an accessor.

private final Theme mTheme = new Theme(); // Should work but leads to NPEs on accesses.

I tried calling this:

private final Theme mTheme = Theme.getRootAsTheme(ByteBuffer.allocate(0)); // Doesn't work

But got an IndexOutOfBoundsException when accessing a property.

Finally, this code worked:

private final Theme mTheme = Theme.getRootAsTheme(ByteBuffer.allocate(4));

IMHO, the first attempt (new YourFlatbuffersTable()) should work without pending NPEs, and getRootAsYourFlatbuffersTable() should be only needed when you have a real flatbuffers binary.

I suggest to address this issue by making the __offset(…) method in Table class check for bb nullability, and return 0 if it is, so the calling accessor/mutator skips to default value/false without trying to call any method on bb.

This would just need one line at the beginning of the method:

protected int __offset(int vtable_offset) {
    if (bb == null) return 0; // This line I added allows empty flatbuffers
    int vtable = bb_pos - bb.getInt(bb_pos);
    return vtable_offset < bb.getShort(vtable) ? bb.getShort(vtable + vtable_offset) : 0;
  }

Also, it should be clearly documented (especially for network apps) that accesses can throw IndexOutOfBoundsException if wrong data it in the ByteBuffer for current implementation, and IMHO, future implementations should have a public static boolean checkIntegrity(ByteBuffer _bb) method generated in each table class, designed to be called before getRootAsYourFlatbuffersTable(ByteBuffer _bb, YourFlatbuffersTable obj) (hence the static keyword) which would prevent someone causing a crash to your app by corrupting network (or shared disk) data. Maybe this is worth a new issue?