fmgasparino / google-gin

Automatically exported from code.google.com/p/google-gin
Apache License 2.0
0 stars 0 forks source link

Null injection of inner class into outer class #113

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?
--------------------------------------
1. Inject an inner class into an outer class through method or field injection. 
The inner class here is intentionally not a static (nested) class.

public class Foo {
    @Inject
    public Bar bar;

    public Foo() {
    }

    public class Bar {
    }
}

What is the expected output?
----------------------------
1. That Gin will create the outer class:
    Foo foo = new Foo();
2. Then Gin will create the inner class:
    Bar bar = foo.new Bar()
3. Gin will inject the inner class instance into the outer class instance:
    foo.bar = bar;
    // or for method injection
    foo.setBar(bar)

Alternatively, Gin should at least raise an error rather than not injecting (or 
injecting a null).

What do you see instead?
------------------------
The @Inject annotated field "bar" in class Foo is null at runtime (not injected 
or maybe injected as null).

What version of the product are you using? On what operating system?
--------------------------------------------------------------------
Gin 1.0 distribution, GWT 2.0.3 - Development Mode, Windows 7

------------------------------------------------
Please provide any additional information below:
------------------------------------------------
It will probably be asked why method and field injection of inner classes is 
desired. I've written up the following rationale:

Because constructor injection of an inner class is impossible (the outer class 
instance must exist before you can instantiate the inner class), injecting such 
dependencies as a field or method is a work-around.

This functionality is desired in order to eliminate the need for anonymous 
inner classes for callbacks, while reducing the amount of code. The following 
would be possible with method or field injection of inner classes:

class Foo extends Composite {
   private final Button button;
   private final ButtonService buttonService;

   @Inject
   public ButtonLabelCallback buttonLabelCallback;

   @Inject
   public ButtonTitleCallback buttonTitleCallback;

   public Foo(Button button, ButtonService buttonService) {
       this.button = button;
       this.buttonLabelService = buttonLabelService;
       buttonService.getLabel(buttonLabelCallback);
       buttonService.getTitle(buttonTitleCallback);
       initWidget(button);
   }

   public class ButtonLabelCallback implements AsyncCallback<String> {
       public void onSuccess(String result) {
           button.setText(result);
       }

       public void onFailure(Throwable caught) {
           GWT.log("Failed to retrieve the button label.", caught);
       }
   }

   public class ButtonTitleCallback implements AsyncCallback<String> {
       public void onSuccess(String result) {
           button.setTitle(result);
       }

       public void onFailure(Throwable caught) {
           GWT.log("Failed to retrieve the button title.", caught);
       }
   }
}

Why not use anonymous inner classes?
------------------------------------
The above separates the anonymous inner class code out into a test-accessible 
structure. Consider the following, which would not be individually testable if 
onSuccess(String) was implemented in an anonymous inner class:

@Test
public void button_label_callback_success() {
   // Given   
   final String label = "Button Label";
   Foo sut = new Foo(mockButton);
   ButtonLabelCallback callback = sut.new ButtonLabelCallback();   

  // When
  callback.onSuccess(label);

  // Verify
  assertEquals(label, mockButton.getText());
}

Why not use a static (nested) inner class?
------------------------------------------
Using a nested class requires that all fields in the outer class be passed as 
constructor arguments. This requires the nested class to contain duplicates of 
all the fields and an explicit constructor which creates unnecessary complexity.

Why use injection rather than directly instantiating the inner class?
---------------------------------------------------------------------
If the inner classes are injected, this means they can isolated from the rest 
of the code and mocked during unit tests.

Original issue reported on code.google.com by np...@cisco.com on 10 Jul 2010 at 7:05

GoogleCodeExporter commented 9 years ago
I made the error in my example of using field injected resources in the 
constructor - which are of course not yet injected:

   @Inject
   public ButtonLabelCallback buttonLabelCallback;

   @Inject
   public ButtonTitleCallback buttonTitleCallback;

   public Foo(Button button, ButtonService buttonService) {
       this.button = button;
       this.buttonLabelService = buttonLabelService;
       buttonService.getLabel(buttonLabelCallback);  <-- Always null
       buttonService.getTitle(buttonTitleCallback);  <-- Always null
       initWidget(button);
   }

So excluding that situation, instead try to imagine using the ButtonService 
after all injections occurred with the two inner class callbacks.

Regards

Original comment by np...@cisco.com on 10 Jul 2010 at 7:55

GoogleCodeExporter commented 9 years ago
Gin does not support inner (as opposed to nested) classes. It probably never 
will since Guice doesn't either: 
http://code.google.com/docreader/#p=google-guice&s=google-guice&t=FrequentlyAske
dQuestions

I'm surprised you didn't get a Guice error complaining about this binding in 
your test, might investigate that later. But the main request - allow use of 
inner classes - is something we can't do.

Original comment by aragos on 12 Jul 2010 at 5:05

GoogleCodeExporter commented 9 years ago
No error at all in development mode, but only when the field is uninitialized. 
If I did:

   private final ButtonService buttonService = new ButtonLabelCallback();

   @Inject
   public ButtonLabelCallback buttonLabelCallback;

Then I get an error. If I do a GWT compile either way, then I get a the same 
mysterious error as: http://code.google.com/p/google-gin/issues/detail?id=99#c3

I realize this is not a forum for Guice in general, but does anyone have any 
insight into why they are not supporting inner classes for field or method 
injection?

Original comment by np...@cisco.com on 12 Jul 2010 at 5:33

GoogleCodeExporter commented 9 years ago
The reason is most likely that the enclosing might not be instantiable: if 
there is no default constructor or the constructor is inaccessible, the 
enclosing class cannot be created, therefore the inner class cannot use a newly 
created instance of the enclosing class.

Original comment by aragos on 19 Jul 2010 at 5:14