@ProvidedBy doesn't work with enums #295

Activity

1. gissuebot commented on Jul 7, 2014

   

   gissuebot

   on Jul 7, 2014

   Author

   More actions

   From limpbizkit on April 26, 2009 14:46:04

   (No comment was entered for this change.)

   Labels: Priority-Low
2. gissuebot commented on Jul 7, 2014

   

   gissuebot

   on Jul 7, 2014

   Author

   More actions

   From Maaartinus on March 04, 2011 16:20:51

   This bug is over 2 years old, and AFAIK, the fix is a (less then) one-liner. There's a (possibly useless) test in InjectorImpl.createUninitializedBinding commented as "Don't try to inject arrays, or enums." - the probable reason for this is the wrong assumption that it makes no sense at all. The test only makes it fail a bit faster in case of something is wrong, and may IMHO be completely omitted. Could you please do it?

   Attachment: gist
      0001-Make-ProvidedBy-work-with-enums-issue-295.patch
3. gissuebot commented on Jul 7, 2014

   

   gissuebot

   on Jul 7, 2014

   Author

   More actions

   From rkapsi on September 25, 2011 12:01:48

   Here's an use-case if you need one. I have an enum that defines the various runtime environments of my software and I use the system properties to define the environment at startup. I'm currently using Module but @ProvidedBy would be a much cleaner approach.

   @ProvidedBy(EnvProvider.class)
   public enum Env {
     DEVELOPMENT,STAGING,PRODUCTION;

     public static Env environment() {
       String env = System.getProperty("my.env");
       if (env != null) {
         return Env.valueOf(env.toUpperCase());
       }
       return DEVELOPMENT;
     }

     public static class EnvProvider implements Provider<Env> {
       public Env get() {
         return Env.environment();
       }
     }
   }
4. gissuebot commented on Jul 7, 2014

   

   gissuebot

   on Jul 7, 2014

   Author

   More actions

   From Maaartinus on December 16, 2013 01:38:40

   I'd propose something like

   public enum Env {
     @DefaultInstance DEVELOPMENT,
     STAGING,
     PRODUCTION;
   }

   but I can see that the Provider-based approach is much more general (and needs no new annotation). My use case is exactly the same. In a few days we can celebrate the 5th birthday of this bug.
5. Maaartinus commented on Jul 17, 2014

   

   Maaartinus

   on Jul 17, 2014

   More actions

   Yes, let's celebrate! And no, I don't mind talking to myself (as long it's not too often). My above proposal is actually a different syntax for

   @ImplementedBy(Env.DEVELOPMENT)
   public enum Env {
     DEVELOPMENT,
     STAGING,
     PRODUCTION;
   }
   

   So it looks like both @ImplementedBy and @ProvidedBy make sense for enums.
6. sameb commented on Jul 17, 2014

   

   sameb

   on Jul 17, 2014

   Member

   More actions

   For @ImplementedBy, that looks like it's trying to provide a default value for Env. The better way to do that would be using the OptionalBinder with a default value.

   I agree it makes sense for @ProvidedBy, though. Is the above patch still valid? If so, can you send a pull request with it?
7. sameb commented on Aug 7, 2014

   

   sameb

   on Aug 7, 2014

   Member

   More actions

   Fixed in 1285790.
8. sameb

   closed this as completedon Aug 7, 2014
9. Maaartinus commented on Aug 8, 2014

   

   Maaartinus

   on Aug 8, 2014

   More actions

   For @ImplementedBy, that looks like it's trying to provide a default value for Env.

   I see I was wrong as the argument of @ImplementedBy must be a class, so a different annotation would be needed.

   The better way to do that would be using the OptionalBinder with a default value.

   Sounds good, except in simple cases when I want it to work without any configuration. But @ProvidedBy solves it (although a bit verbosely).