4 Replies Latest reply on Oct 14, 2009 10:58 AM by flavia.rainone

    ModifierInfo: enum or interface?

    flavia.rainone

      When running the deployers-vfs tests against the trunk version of jboss-reflect, I stumbled accross a few failures related to ModifierInfo class.

      It looks like those failures were introduced when ModifierInfo interface was transformed into an enum:
      http://www.jboss.org/index.html?module=bb&op=viewtopic&t=148169&postdays=0&postorder=asc&start=30

      At first, I also thought it was a good idea making ModifierInfo an enum, but now I'm not so sure. The first point is that it implies in creating a single instance of every possible combination of modifiers, and the current version of ModifierInfo lacks several of them (for example, PUBLIC_STATIC_FINAL).

      Apart from that, another problem I found was that the method ModifierInfo.getNewModifier looks for an instance with the same number as the parameter modifiers:

      public static ModifierInfo getNewModifier(int modifiers)
       {
       for(ModifierInfo modifier : values())
       if(modifier.getModifiers() == modifiers)
       return modifier;
      
       throw new RuntimeException("Couldnt find Modifier for: "+modifiers);
       }

      The problem is that sometimes the JVM (sun jdk1.6) returns numbers such as 4101, which represent the static modifier only, and are different from the modifiers value contained in the instance Modifier.STATIC, which is 1. So, when I ran accross this kind of failure, I fixed the method above, making it check against every possible modifier and building the expected equivalent modifier value:
      int m = 0;
       if (Modifier.isStatic(modifiers))
       {
       m += Modifier.STATIC;
       }
       if (Modifier.isAbstract(modifiers))
       {
       m += Modifier.ABSTRACT;
       }
       if (Modifier.isFinal(modifiers))
       {
       m += Modifier.FINAL;
       }
       if (Modifier.isInterface(modifiers))
       {
       m += Modifier.INTERFACE;
       }
       if (Modifier.isNative(modifiers))
       {
       m += Modifier.NATIVE;
       }
       if (Modifier.isPrivate(modifiers))
       {
       m += Modifier.PRIVATE;
       }
       if (Modifier.isProtected(modifiers))
       {
       m += Modifier.PROTECTED;
       }
       if (Modifier.isPublic(modifiers))
       {
       m += Modifier.PUBLIC;
       }
       if (Modifier.isStrict(modifiers))
       {
       m += Modifier.STRICT;
       }
       if (Modifier.isSynchronized(modifiers))
       {
       m += Modifier.SYNCHRONIZED;
       }
       if (Modifier.isTransient(modifiers))
       {
       m += Modifier.TRANSIENT;
       }
       if (Modifier.isVolatile(modifiers))
       {
       m += Modifier.VOLATILE;
       }
       for(ModifierInfo modifier : values())
       if(modifier.getModifiers() == m)
       return modifier;


      IMO, that fix doesn't look very nice, and if you call MethodInfo.getModifiers().getModifers() the value you end up with may be different from the original modifiers (take, for example, the case of 4101 vs. 1 I mentioned previously). I don't know if this is an issue, but it breaks several test checks.

      Even with all those fixes, I'm getting failures now on deployers-vfs tests like the one below:
      Caused by: java.lang.NoSuchMethodError: org.jboss.reflect.spi.ClassInfo.getModifiers()I
       at org.jboss.xb.builder.runtime.CollectionPropertyHandler.<init>(CollectionPropertyHandler.java:72)
       at org.jboss.xb.builder.runtime.CollectionPropertyWildcardHandler.<init>(CollectionPropertyWildcardHandler.java:51)
       at org.jboss.xb.builder.JBossXBNoSchemaBuilder.generateType(JBossXBNoSchemaBuilder.java:1164)


      Before I try to fix it, I need to know if we are going to stick with ModifierInfo enum or with ModifierInfo interface.

      So, Ales, any ideas?

        • 1. Re: ModifierInfo: enum or interface?
          alesj

           

          "flavia.rainone@jboss.com" wrote:

          Before I try to fix it, I need to know if we are going to stick with ModifierInfo enum or with ModifierInfo interface.

          If the fix for this is to revert it back to ModifierInfo as an interface,
          then that's exactly what I would do.

          The error you're getting looks like a compatibility mismatch between XB and Deployers.
          I guess the revert should fix that as well.


          • 2. Re: ModifierInfo: enum or interface?
            flavia.rainone

             

            "alesj" wrote:
            "flavia.rainone@jboss.com" wrote:

            Before I try to fix it, I need to know if we are going to stick with ModifierInfo enum or with ModifierInfo interface.

            If the fix for this is to revert it back to ModifierInfo as an interface,
            then that's exactly what I would do.


            There are two possible fixes: reverting or updating dependencies such as the one in org.jboss.xb.builder.runtime.CollectionPropertyHandler.

            On one hand, updating all dependencies at this point may be too much hassle. Plus, the fix I added to getNewModifier method can generate unnecessary overhead. This method is called everytime a new ClassInfo, MethodInfo, ConstructorInfo and FieldInfo is created, to fill in the modifier field.

            On the other hand, the interface version has a drawback: it forces you to recalculate if it is static, for example, for as many times as method isStatic is called. But I don't know at what frequency the modifiers are checked, so I can't say how it compares to the overhead of getNewModifier method.

            Given all that, I vote for reverting back to ModifierInfo interface.


            • 3. Re: ModifierInfo: enum or interface?
              flavia.rainone

              I'm reverting to ModifierInfo interface.

              • 4. Re: ModifierInfo: enum or interface?
                flavia.rainone