Using google guice for dependency injection.

[ddscharfe]

As talked about in the last cdt call, I'd like to discuss whether to use DI in cdt-lsp and how to use it.

What I generally like about using DI is that it makes testing easier (e.g. by avoiding static methods) and helps sharing code between classes. On the other hand it adds some complexity and makes dependencies less obvious, which some people don't like.

I setup an example where I modified ToggleSourceAndHeaderCommandHandler to use an external implementation to get the uri for an editor part. As there is no test (yet) for this, using DI there doesn't bring too much benefits, but I think it is enough to show how we could use DI in cdt-lsp.
Note that there is some initial boilerplate (e.g. org.eclipse.cdt.lsp.di.AbstractGuiceAwareExecutableExtensionFactory which I copied from xtext and the Injector creation in the activators). However once setup, I think this is an easy way to use DI.

See: master...ddscharfe:eclipse-cdt-lsp:di-example
The "interesting" part is in c001fff.

@jonahgraham @ghentschke @ruspl-afed

[ruspl-afed]

Thank you for your example @ddscharfe .
Yes, I understand, we can override the extension factory right in the contribution to intercept the construction of an object.

Please note, that for command handler in particular and for some other UI objects we can use "native" Eclipse DI.

It would be great if you can extend your example to let us see:

1. inversion of control
   the interface and its usage should be both in "org.eclipse.cdt.lsp" while its implementation should be magically supplied from "org.eclipse.cdt.lsp.editor.ui", who is not visible from "org.eclipse.cdt.lsp"

2. benefits, i.e. some skeletons that demonstrates how

it makes testing easier (e.g. by avoiding static methods) and helps sharing code between classes.

[jonahgraham]

This looks ok to me. The bindings as written is ok to me, and the tooling works. There is a little magic, but no worse and probably better than the magic of adapters in Eclipse, but not quite as nice as a well written extension.

The change as you have presented it makes getUri properly testable which is a good way forward.

Do you see using the guice as an internal implementation detail, or does it apply to extending the code too? On the former it doesn't really affect the API and makes the code testable which is a good thing.

When it comes to third-parties extending the CDT code with Guice I am less clear on how it would best work. e.g does your suggestion include changing how server providers are done?

cdt-lsp/bundles/org.eclipse.cdt.lsp/plugin.xml

Line 4 in 71bc4c2

<extension-point id="serverProvider" name="C/C++ Language Server" schema="schema/serverProvider.exsd"/>

[ruspl-afed]

Could you provide an example, maybe doing the same as I did in mine, but with OSGI or Eclipse DI?

Yes, this is fair. Please give me some time, I`ll get back here.

I think we can agree on that testing code which uses injected dependencies is easier to test in general, right?

I would say that code that operates with instances of interfaces instead of static calls is easier to test. I don't see strong correlation with injection here, but I may be wrong.

[ruspl-afed]

Here is example for OSGi and E4 DI, it works for me from workspace sources

• OSGi part is how it should be and has its own value without E4 part
• E4 DI part needs more polishing
  • to avoid duplications of UI contributions between E3 and E4
  • to create proper menu contribution

But I hope it is enough to demonstrate these technologies in action @ghentschke @ddscharfe @jonahgraham

[ddscharfe]

Thanks for providing an example @ruspl-afed, interesting to see how this works.

Your example instantly raises some questions about how certain things can be done (e.g. assisted inject), but this might go a little to deep for this discussion. Btw there is an OSGi section in the guice documentation: https://github.com/google/guice/wiki/OSGi. Honestly I don't really understand it due to my lack of OSGi knowledge.

I'm open to which DI technology we use (if any).

[ruspl-afed]

My main argument is: we need to try what the Platform offers before introducing another third-party technology.

[ghentschke]

My main argument is: we need to try what the Platform offers before introducing another third-party technology.

I agree

[ghentschke]

I've made similar experiences with with DI as @ddscharfe. We are using javax.inject.Inject and the
org.eclipse.e4.core.di.annotations.Creatable for the Creatable annotation.

I cannot say which injection framework is better.

[ruspl-afed]

I'm also not saying that one is better than another. Environment is important.
For things like XText that are designed to work in a "standalone" mode I would select Guice,
And for things that depend heavily on a "running Platform", I wouldn't introduce another dimension of DI.