Remove synthetic accessor from generated code about dataenum HOT 4 CLOSED

Doing a little thinking about how you might configure this behaviour, I've come up with some options:

1. Locally, using a field in the @DataEnum annotation
2. Locally, using a separate annotation (along the lines of @ConstructorVisibility(PRIVATE))
3. Globally, using the -A javac option. (javac -ACONSTRUCTOR_VISIBILITY=PACKAGE_PRIVATE ...)
4. Globally, using an environment variable/system property (CONSTRUCTOR_VISIBILITY=PRIVATE javac ...).

I think the way you'd like to configure this is globally. There seems to be very little value in doing it locally, because the one reason to avoid synthetic constructors is to reduce the method count for Android. And the ways of configuring the annotation processor behaviour are quite obscure and hard to use (from build files), meaning that most people are unlikely to use them.

So that makes me not like the configurability option so much. I guess a large Android app using DataEnum with Mobius might have several hundred DataEnum cases in it with associated synthetic methods. Not a whole lot, but also not quite ignorable. I'm leaning towards changing this and make the case constructors package-private, without making it configurable, or perhaps, making it possible to make constructors private through a specific annotation (could even be applied to a (parent) package via package-info.java to make it nearly global).

from dataenum.

Thanks for the report! Could you quantify/clarify 'big saving'? It seems like the penalty comes in the form of a) an extra method for each dataenum_case, and b) a layer of indirection (which should be inlined by JIT compilation if hot, I would think). On the other hand, making access package-private leads to opening up a slightly less readable API/two ways of constructing case instances. I find it hard to see an example of when the penalties for the synthetic methods would be large, but I'd be happy to be corrected.

from dataenum.

I've done some digging into the generated classes, and I can confirm that there is an extra synthetic constructor generated for each case. Here's some hacky code, added as a test case to the OuterIntegrationTest class:

  @Test
  public void printMethodsInGeneratedCode() {
    printMethods(Testing.class);
    printMethods(One.class);
    printMethods(Two.class);
  }

  private void printMethods(Class<?> oneClass) {
    System.out.println();
    System.out.println("Methods in class: " + oneClass.getCanonicalName());
    System.out.println("declared methods:");
    for (Method m : oneClass.getDeclaredMethods()) {
      System.out.println(
          Modifier.toString(m.getModifiers())
              + ((m.getModifiers() & 0x00001000) != 0 ? "synthetic" : "")
              + " "
              + m.getName()
              + "("
              + Arrays.toString(m.getParameters())
              + ")");
    }

    System.out.println("===================");
    System.out.println("constructors:");
    for (Constructor c : oneClass.getDeclaredConstructors()) {
      System.out.println(
          Modifier.toString(c.getModifiers())
              + ((c.getModifiers() & 0x00001000) != 0 ? "synthetic" : "")
              + " "
              + c.getName()
              + "("
              + Arrays.toString(c.getParameters())
              + ")");
    }
  }

The output looks like:

Methods in class: com.spotify.dataenum.it.Testing.One
declared methods:
public equals([java.lang.Object arg0])
public toString([])
public hashCode([])
public final match([com.spotify.dataenum.function.Consumer<com.spotify.dataenum.it.Testing$One> arg0, com.spotify.dataenum.function.Consumer<com.spotify.dataenum.it.Testing$Two> arg1, com.spotify.dataenum.function.Consumer<com.spotify.dataenum.it.Testing$Three> arg2])
public final map([com.spotify.dataenum.function.Function<com.spotify.dataenum.it.Testing$One, R_> arg0, com.spotify.dataenum.function.Function<com.spotify.dataenum.it.Testing$Two, R_> arg1, com.spotify.dataenum.function.Function<com.spotify.dataenum.it.Testing$Three, R_> arg2])
private staticsynthetic $$$reportNull$$$0([int arg0])
public final i([])
===================
constructors:
private com.spotify.dataenum.it.Testing$One([int arg0])
synthetic com.spotify.dataenum.it.Testing$One([int arg0, com.spotify.dataenum.it.Testing$1 arg1])

The second constructor goes away if the generation code is changed to use package-private rather than private constructors. I'm not sure this is a tradeoff you always want to make, given that this code lives in a user package. It could be a reason to start introducing configuration parameters to the code generation.

from dataenum.

Agreed, config is the way to go. In android synthetic accessors are an issue but they are ok for other environments. Most of the Android libraries have removed synthetic accessor. Some of the static code analyzers are able to detect those.

from dataenum.