1 2 3 Previous Next 31 Replies Latest reply on Mar 7, 2008 5:49 PM by Adrian Brock

    enums, generics and other animals

    Adrian Brock Master

      I don't understand why there is an InjectOption and an InjectionOption?

      The reason the ControllerState and ControllerMode are the way they are
      is because the MC was originally written on JDK1.4
      so they couldn't be written as enums.

      Actually, since ControllerState is extensible (you can create your own states)
      it can't be an Enum.

      All of these should just be enums throughout

      Cardinality
      FromContext
      InjectionOption (already is InjectOption???)
      AutowireType

      Since we are doing 2.0.0 we could also decde to change ControllerMode to an Enum
      as well?

      Also the FromContext is a misuse of generics.
      The purpose of generics is to produce compile time errors, the way this class
      is used, it is simply not possible for the compiler to do so.

      It looks to me like the generic is only here to avoid having to manually code
      the check/cast of ControllerContext -> KernelControllerContext
      but you should do that by hand anyway to give a better error message than CCE

      e.g. MetaDataFromContext

       public MetaData internalExecute(ControllerContext context)
       {
       if (context instanceof KernelControllerContext == false)
       throw new IllegalArgumentException("Cannot get metadata from context " + context);
       KernelControllerContext kernelContext = (KernelControllerContext) context;
       return kernelContext.getScopeInfo().getMetaData();
       }
      


      In fact, this is wrong anyway, since you can get a ScopeInfo/Metadata
      from any context. ;-)

        • 1. Re: enums, generics and other animals
          Ales Justin Master

           

          "adrian@jboss.org" wrote:
          I don't understand why there is an InjectOption and an InjectionOption?

          The non enum version was first added.
          But when the annotation support kicked in, I didn't want to rely on the String, so the enum was added, but the old one was never removed. :-(

          You can also see that the FromContext suffers from the same syndrome.
          Where Inject(ion)Option should really be enum, since it only has finite number of usages, the FromContext due to its Dynamic case cannot be.

          So, it the case you use annotations, you cannot get the DynamicFromContext.

          The non-enum FromContext is really an impl detail.
          And perhaps to avoid confusion, it can be renamed.

          "adrian@jboss.org" wrote:

          All of these should just be enums throughout

          Cardinality
          FromContext
          InjectionOption (already is InjectOption???)
          AutowireType

          The Cardinality and FromContext cannot be enums.

          The Cardinality can be whatever, not just those 4 usual usages.
          e.g. 2..7, 5..n, etc

          For FromContext see DynamicFromContext.

          "adrian@jboss.org" wrote:

          Since we are doing 2.0.0 we could also decde to change ControllerMode to an Enum as well?

          Since I'll change a few of them anyway, I don't see why not this one as well.

          "adrian@jboss.org" wrote:

          Also the FromContext is a misuse of generics.
          The purpose of generics is to produce compile time errors, the way this class is used, it is simply not possible for the compiler to do so.

          It looks to me like the generic is only here to avoid having to manually code the check/cast of ControllerContext -> KernelControllerContext
          but you should do that by hand anyway to give a better error message than CCE.

          I'm already doing validation, so the error msg is useful.
          But you're right, this is a misuse. A handy one, but a misuse. :-)
          I'll remove the generics.

          • 2. DynmanicFromContext was Re: enums, generics and other animal
            Adrian Brock Master

             

            "alesj" wrote:
            "adrian@jboss.org" wrote:
            I don't understand why there is an InjectOption and an InjectionOption?

            So, it the case you use annotations, you cannot get the DynamicFromContext.
            ...
            For FromContext see DynamicFromContext.


            What is that rubbish? :-)

            Why are you exposing "random" things?

            How do we know what people have used in the api if they can just
            select parts of it using reflection?
            You might as well let people write
            <property><script type="javascript">do what you like here but its not very declarative
            or guaranteed to work in future versions</script></property>
            


            It also exposes things that are "private interfaces"

            // Intended for the user
            public interface ControllerContext {}
            
            public class MyControllerContext implements ControllerContext,
             ImplementDetails // Not intended for the user and subject to change without notice
            {
            }
            


            • 3. Re: enums, generics and other animals
              Adrian Brock Master

               

              "alesj" wrote:

              The Cardinality can be whatever, not just those 4 usual usages.
              e.g. 2..7, 5..n, etc


              ok.

              • 4. Re: DynmanicFromContext was Re: enums, generics and other an
                Ales Justin Master

                 

                "adrian@jboss.org" wrote:
                "alesj" wrote:
                "adrian@jboss.org" wrote:
                I don't understand why there is an InjectOption and an InjectionOption?

                So, it the case you use annotations, you cannot get the DynamicFromContext.
                ...
                For FromContext see DynamicFromContext.


                What is that rubbish? :-)

                Deleted. :-(

                It looked cool at that time.
                So that I wouldn't have to write new impl every time Diesler wanted to pull some detail from the underlying component. :-)

                So, another enum coming up.

                • 5. Re: DynmanicFromContext was Re: enums, generics and other an
                  Adrian Brock Master

                   

                  "alesj" wrote:

                  It looked cool at that time.


                  Beware of "cool features". :-)

                  They'll come back to haunt you when you need to make changes
                  and realise it is no longer supportable in its current form because you did a "quick and dirty".

                  Or in this case, DONT realise you are removing a feature because
                  there's no contract or test for it. :-)

                  Don't do a feature unless it is obviously correct, but even then
                  only when somebody asks for (needs) it.

                  It's less to support, maintain, fix and answer annoying questions
                  (from users that can't be bothered to read the docs) about in future. :-)

                  • 6. Re: enums, generics and other animals
                    Ales Justin Master

                    Where to keep all this enums?
                    Since they are now scattered all over the place:

                    ControllerMode --> org.jboss.dependency.spi // OK, this one is here to stay
                    FromContext --> org.jboss.beans.metadata.plugins
                    AutowireType --> org.jboss.beans.metadata.spi
                    InjectOption --> org.jboss.beans.metadata.api.annotations

                    The idea with annotations was to keep them in an 'api' package + have separate artifact for them, so user can use just that for his beans.

                    And since now both, beans metadata and annotations, use the same enums, we should find some appropriate place. :-)

                    Perhaps org.jboss.beans.metadata.api.enums?
                    But there is the AutowireType which is part of spi? :-(
                    Moving all but that one there?

                    • 7. Re: enums, generics and other animals
                      Adrian Brock Master

                      If they are used by the annotations then they should be in the api.

                      We should really look to move all the annotations (MC, Managed, etc.)
                      to the jboss-integration project as seperate subprojects
                      so people can consume just them for compiles as they need them

                      No need to do this straightaway though. You'll conflict with what I'm doing
                      if you do that now. ;-)

                      • 8. Re: enums, generics and other animals
                        Ales Justin Master

                         

                        "adrian@jboss.org" wrote:
                        If they are used by the annotations then they should be in the api.

                        Even the AutowireType?
                        Moving it out of spi?

                        • 9. Re: enums, generics and other animals
                          Adrian Brock Master

                           

                          "alesj" wrote:
                          "adrian@jboss.org" wrote:
                          If they are used by the annotations then they should be in the api.

                          Even the AutowireType?
                          Moving it out of spi?


                          Yes. If it is used by the annotations then people need it to compile.

                          Also, even for things like ControllerState you can create a helper class.
                          e.g.
                          public interface MicrocontainerConstants
                          {
                           String CREATE = "Create";
                           String INSTANTIATED = "Instantiated";
                          }
                          


                          That way people can reference the constants in their annotations
                          instead of suffering from typos for the common values. ;-)

                          • 10. Re: enums, generics and other animals
                            Ales Justin Master

                             

                            "adrian@jboss.org" wrote:

                            Also, even for things like ControllerState you can create a helper class.
                            e.g.
                            public interface MicrocontainerConstants
                            {
                             String CREATE = "Create";
                             String INSTANTIATED = "Instantiated";
                            }
                            


                            That way people can reference the constants in their annotations
                            instead of suffering from typos for the common values. ;-)

                            I though using interfaces to get access to constants was anti-pattern. ;-)

                            • 11. Re: enums, generics and other animals
                              Adrian Brock Master

                               

                              "alesj" wrote:

                              I though(t) using interfaces to get access to constants was anti-pattern. ;-)


                              It's only an anti-pattern if you are lazy/stupid and "implement" the interface ;-)

                              The import static and enums have replaced this usage but only if your
                              constants are actually enums, i.e. a fixed set of values.

                              • 12. Re: enums, generics and other animals
                                Ales Justin Master

                                Another refactoring question. :-)
                                Since for xml handling I still need to get the right enum by matching the underlying strings.
                                The thing is that on enums there is already a valueOf method which does similar thing, matching enum's name with input string parameter.
                                But in our case the names do not match exactly the underlying strings, resulting in needing something like getInstance method which iterates over the enums and does a string match.
                                Should I just use the valueOf and fix the strings in our tests?
                                Probably not, since it will break quite some things. :-)

                                • 14. Re: enums, generics and other animals
                                  Adrian Brock Master

                                   

                                  "alesj" wrote:
                                  Another refactoring question. :-)
                                  Since for xml handling I still need to get the right enum by matching the underlying strings.
                                  The thing is that on enums there is already a valueOf method which does similar thing, matching enum's name with input string parameter.
                                  But in our case the names do not match exactly the underlying strings, resulting in needing something like getInstance method which iterates over the enums and does a string match.
                                  Should I just use the valueOf and fix the strings in our tests?
                                  Probably not, since it will break quite some things. :-)


                                  RTFM :-)

                                  You can actually do some pretty weird stuff, so don't overuse it ;-)

                                  @XmlEnum(Integer.class)
                                  public enum OldEnglishMoney
                                  {
                                   @XmlEnumValue("D")
                                   PENNY(1),
                                   @XmlEnumValue("S")
                                   SHILLING(12)
                                   @XmlEnumValue("L")
                                   POUND(240)
                                  }
                                  


                                  1 2 3 Previous Next