Flatbuffers: Support IntDef, Nullable and NotNull annotations for Android java

[LouisCAD]

Hi!

I started using flatbuffers to sync Watch Face themes across Android handset and paired Android Wear devices, and I find there's room for improvement for java implementation:

Flatbuffers enums don't use java enums, which is great according to this guy, but it would be much better if it generated IntDefs for us to get safety lint checks on Android. It would also be nice to get nullable/not nullable annotations on Android.

My suggestion is the following:

1. Add an --android-java option to flatc next to --java one to optimize the code for Android.
2. When flatc in invoked with --android-java, add @Nullable to all non primitive fields and generate @interfaces instead of classes for enum definitions, using @IntDef on them.
3. Generate string constants for enum names only on demand, when an option is added (e.g. --gen-enum-names)

Here's a comparison with the code currently being generated by flatc 1.3 and how I'd like it to write it:

my_schema_snippet.fbs

namespace xyz.louiscad.common.model;

enum ColorType : int {
    ARGB, MD_COLOR_KEY
}

flatc 1.3

// automatically generated, do not modify

package xyz.louiscad.common.model;

public final class ColorType {
  private ColorType() { }
  public static final int ARGB = 0;
  public static final int MD_COLOR_KEY = 1;

  private static final String[] names = { "ARGB", "MD_COLOR_KEY", };

  public static String name(int e) { return names[e]; }
};

desired output

package xyz.louiscad.common.model;

import android.support.annotation.IntDef;

import java.lang.annotation.Documented;
import java.lang.annotation.Retention;

import static java.lang.annotation.RetentionPolicy.SOURCE;

@Documented
@Retention(SOURCE)
@IntDef({ColorType.ARGB, ColorType.MATERIAL_COLOR_KEY})
public @interface ColorType {
    int ARGB = 0;
    int MATERIAL_COLOR_KEY = 1;
}

[ghost]

This seems like a nice suggestion. Is this new code compatible with the old code (i.e. can use ColorType.ARGB as before?) Can Java code that uses these enums and is intended to be platform agnostic work with this? And names[] appears to be gone.

[LouisCAD]

This new code would be compatible with old one on Android, but would not compile without IntDefs which are specific to Android. That's why I suggested this code style to be an option to generate Android optimized java code (--android-java). And yes, names[] is gone because It's not really useful unless you want to display the enum names to users IMHO (from my use in java on Android). That's why I suggested names[] to be generated only if an option such as --gen-enum-names is specified on flatc.

[ghost]

Sure. But I am saying if I have code that reads/writes FlatBuffers, that code will work regardless of wether it is compiled against a class generated with --android-java or without?

If so, go ahead and make a PR. I'd suggest calling the flag just --android, that way if we want to add Android specifics to C++ or whatever, we don't need yet another flag.

Also, please retain names[], as there is another PR ongoing to make it public. We want to keep these strings available on all platforms for uniformity.

[Lakedaemon]

Le 13/07/2016 18:32, Wouter van Oortmerssen a écrit :

Sure. But I am saying if I have code that reads/writes FlatBuffers,
that code will work regardless of wether it is compiled against a
class generated with |--android-java| or without?

IntDef, @Nullable @NotNull, etc.. only affect the generated code and not
the serialized bits.
The java code that is used for Android is slightly different from
standard java code
(because android only uses the java api, not the java binaries, the java
classes/bytecodes are translated to dex/dalvik/art bytecode)

If so, go ahead and make a PR. I'd suggest calling the flag just
|--android|, that way if we want to add Android specifics to C++ or
whatever, we don't need yet another flag.

Also, please retain |names[]|, as there is another PR ongoing to make
it public. We want to keep these strings available on all platforms
for uniformity.

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/google/flatbuffers/issues/3933#issuecomment-232411259,
or mute the thread
https://github.com/notifications/unsubscribe/AAGTB5C1qyvcXKMt44kfHz04lOIAhzoqks5qVRMOgaJpZM4JItqe.

[ghost]

I understand that. I wanted to know what the differences are for the client code using it, if any.

[LouisCAD]

I just forgot to tell that java interfaces (with or without the @ that makes them an annotation) don't support methods, so the names()would have to be ditched, but that would not be a major problem as all fields, including the names array are public in interfaces, which would then allow access to them (but without having to call a method, which is more expensive than just accessing the array on Android anyway)

[ghost]

I don't think we want to ditch name/names, keeping backwards compatibility is somewhat important.

[LouisCAD]

@gwvo But if it's an option, people who would use the --android option would know what to expect and would see how the generated code is if using the names(…)method.

Here's how it woudl look like with --android option:

package xyz.louiscad.common.model;

import android.support.annotation.IntDef;

import java.lang.annotation.Documented;
import java.lang.annotation.Retention;

import static java.lang.annotation.RetentionPolicy.SOURCE;

@Documented
@Retention(SOURCE)
@IntDef({ColorType.ARGB, ColorType.MATERIAL_COLOR_KEY})
public @interface ColorType {
    int ARGB = 0;
    int MATERIAL_COLOR_KEY = 1;

    String[] names = { "ARGB", "MD_COLOR_KEY", };
}

I'd then just need a ColorType.names[ColorType.ARGB]call to get the name. So basically just replace parenthesis with brackets.

[ghost]

I guess that may be acceptable. The problem is with code that needs to work on both Android and desktop, now can't access names in a portable way.

Any other Android/Java users have an opinion?

[LouisCAD]

I personally think it's not a problem to generate java code for non Android runtime, and again for Android runtime. Non Android java runtimes will probably be servers, and it's as if the server was not running java, you generate for the language that the runtime understand. Android is quite different from other JVMs.

I mean, users should not run some code designed for Android on non Android runtimes, and vice-versa, hence the suggested --android option to exclude if the code is not going to run on Android.

[Lakedaemon]

Le 15/07/2016 12:52, Louis Cognault a écrit :

I personally think it's not a problem to generate java code for non
Android runtime, and again for Android runtime. Non Android java
runtimes will probably be servers, and it's as if the server was not
running java, you generate for the language that the runtime
understand. Android is quite different from other JVMs.

This makes sense to me.

Also, the aim of introducing a base class for code generators was to
allow reuse/avoid code duplication and simplify the code base.

In this aspect, it might be interesting to have a class that holds most
of the common java stuff
and to just override a few methods to fine tune the code generator for :

a) java with true enums (some people might want the safety)
b) java for android (some people might want the nice
Nullable/NotNull/IntDef features)
...
c) java with int instead of enums (some people might want the performance)
d) who knows with java 9, java 10 coming it might be interesting to
change the public api to make flatbuffers a lot easier/safer to use (I
saw an effort to make the api more pythonic, the kotlin api is
definately simpler with just 1 line to create a complex flatbuffer...)

I mean, users should not run some code designed for Android on non
Android runtimes, and vice-versa, hence the suggested |--android|
option to exclude if the code is not going to run on Android.

I can see the point of @gwo though :
what if somebody want to generate C++ code for the android ndk ?
They might want to have nice android features kbaked into their
generated code

and --cpp --android avoids a switch compared to --cpp-android

Also, cpp/java is for language and android for a platform (somewhat).
These might be 2 orthogonal concerns

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
https://github.com/google/flatbuffers/issues/3933#issuecomment-232921745,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAGTBwq0KDc-6wLlthzY0Qo_fozkv1nyks5qV2aCgaJpZM4JItqe.

[ghost]

Not sure I agree.

Say I write some code that does processing of FlatBuffer data, and it does by reading a FlatBuffer thru the generated Java API. Now I want to run this processing both in my Android app and on the server. Is it going to fail to compile on one of the other because the way enums are generated? That doesn't sound ideal to me.

[LouisCAD]

@gwvo Say you try to run an Android library on a server… should it work?

[ghost]

I think I should be able to write code that deals with FlatBuffer data, and have it compile on Android and non-Android Java platforms alike, yes.

[LouisCAD]

To do this, you would just have to omit the --android option. I really don't understand your concern about providing the ability to optimize the code for Android, a mobile platform, which is very different from other java environments. Please, if there's a real reason why you wouldn't want Flatbuffers to be optimized for Android, tell us.
Again a comparison between code vs platform: iOS libraries are developed in Objective-C and/or Swift, and nobody complains they don't work on other Objective-C systems such as macOS, watchOS or Objective-C compatible Linux distributions, so I don't see why should be different for Android.

[LouisCAD]

And of course, isn't the flatbuffers .fbs file that is supposed to be platform agnostic? The code can be generated at any time from command line, so ne need to on top of this, requiring the code to be platform agnostic too.

[ghost]

Ok, I suppose if people want to write portable code they'll just have to use the current API.

[LouisCAD]

So I can consider this feature request accepted and start looking how I can implement it?

[ghost]

Sure.