I'm reposting this in github https://github.com/google/guice/issues/1636, 
as this forums seems to miss some love.

Please, answer there.

On Monday, June 13, 2022 at 5:37:00 PM UTC+2 Alberto wrote:

> Dear List,
> I'm trying to debug an issue (
> https://github.com/serenity-bdd/serenity-gradle-plugin/issues/9) in a 3rd 
> party framework that uses Guice. The problem has to do with the integration 
> of the framework (serenity-bdd) in a gradle plugin, where some instance 
> isn't recreated when needed.
> Inspecting the code, I can summarize what I fell is a bad practice. The 
> code has a Singleton more or less like this (see in the sources: 
> https://github.com/serenity-bdd/serenity-core/blob/82cd173792d9f7898791cab0fe8d3a775bfb2825/serenity-model/src/main/java/net/thucydides/core/guice/Injectors.java
> ):
> public class Injectors {
>     private static Injector injectors;
>     public static synchronized getInjector() {
>         if (injector == null) {
>             injector = Guice.createInjector (new MyModule());
>         }
>          return injector;
>     }
> }
> and then uses Injectors.getInjector() to get instances of classes 
> everywhere through the code. For instance (see in the sources: 
> https://github.com/serenity-bdd/serenity-core/blob/82cd173792d9f7898791cab0fe8d3a775bfb2825/serenity-core/src/main/java/net/serenitybdd/core/pages/PageUrls.java#L118-L124
> ):
> private static String namedUrlFrom(String annotatedBaseUrl) {
>     String pageName = 
> annotatedBaseUrl.substring(NAMED_URL_PREFIX.length());
>     EnvironmentVariables environmentVariables = 
> Injectors.getInjector().getInstance(EnvironmentVariables.class);
>     return EnvironmentSpecificConfiguration.from(environmentVariables)
>         .getOptionalProperty(pageName)
>         .orElse(environmentVariables.getProperty(pageName));
> }
> There are also many places throughout the code where constructors uses the 
> Injector to initialize its fields. For example (see in the sources: 
> https://github.com/serenity-bdd/serenity-core/blob/82cd173792d9f7898791cab0fe8d3a775bfb2825/serenity-core/src/main/java/net/serenitybdd/core/annotations/locators/SmartAnnotations.java#L165-L167
> ):
> public SmartAnnotations(Field field, MobilePlatform platform) {
>     this(field, platform, 
> WebDriverInjectors.getInjector().getInstance(CustomFindByAnnotationProviderService.class));
> }
> I have no experience using Guice, and no experience with the framework, so 
> I cannot say if it's very wrong or acceptable. But this (anti?)pattern is 
> giving me hard times trying to debug. As it is now, many functions are 
> quite compact, but I guess we are loosing many benefits of Guice, and I'm 
> not sure if this is a legal use at all.
> I have began to rewrite the code and prepare a Pull Request, but it's 
> extremely costly and I'm not sure that I will do better than the current 
> code. So the question for the experts: Is this so bad that it would require 
> a complete rewrite of the relevant code? Am I missing something? 
> Best regards,
> Alberto

You received this message because you are subscribed to the Google Groups 
"google-guice" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to google-guice+unsubscr...@googlegroups.com.
To view this discussion on the web visit 

Reply via email to