Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

@ProvidedBy doesn't work with enums #295

Closed
gissuebot opened this issue Jul 7, 2014 · 8 comments
Closed

@ProvidedBy doesn't work with enums #295

gissuebot opened this issue Jul 7, 2014 · 8 comments

Comments

@gissuebot
Copy link

From limpbizkit on December 29, 2008 15:22:36

Thierry reports that @ProvidedBy doesn't work with enums. @ImplementedBy and enums doesn't
make sense, but this should definitely work.

Original issue: http://code.google.com/p/google-guice/issues/detail?id=295

@gissuebot
Copy link
Author

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

(No comment was entered for this change.)

Labels: Priority-Low

@gissuebot
Copy link
Author

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

@gissuebot
Copy link
Author

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();
    }
  }
}

@gissuebot
Copy link
Author

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.

@Maaartinus
Copy link

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.

@sameb
Copy link
Member

sameb commented Jul 17, 2014

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?

@sameb
Copy link
Member

sameb commented Aug 7, 2014

Fixed in 1285790.

@sameb sameb closed this as completed Aug 7, 2014
@Maaartinus
Copy link

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).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants