corona-warn-app / cwa-app-android

Native Android app using the Apple/Google exposure notification API. The CWA development ends on May 31, 2023. You still can warn other users until April 30, 2023. More information:
https://coronawarn.app/en/faq/#ramp_down
Apache License 2.0
2.44k stars 495 forks source link

Introduction of a Dependency Injection Framework #55

Closed R4md4c closed 4 years ago

R4md4c commented 4 years ago

Current Implementation

Right now, The code uses object Singletons and static methods to access shared data and execute code e.g. [1], [2] and in many more files.

This would make testing so hard as it will make the experience for developers and contributors unpleasant as they would spend more time in writing the mock code rather than testing their actual work. e.g. [3]

Suggested Enhancement

Introduction of a DI framework that will make writing a testable, maintainable and extensible code achievable. The selection of which framework to use is up to the maintainers.

Expected Benefits

These are the points on top of my head, I might be for sure missing some.

MrIceman commented 4 years ago

You don't even need a DI library, most of the time Dagger2 or Koin produce more Boilerplate code than a simple singleton Factory would do

kedar1607 commented 4 years ago

What do you guys think about using Android Dagger instead of standard dagger. This is optimized especially for android and is much less verbose than standard dagger. For example you don't need to manage subcomponents yourself, @ContributesAndroidInjector does all that magic for you.

marcmuschko commented 4 years ago

Hi @R4md4c,

thank you for this suggestion! We are currently evaluating if and when to include this as it would require quite a bit of effort on our side. I will leave the issue open and keep you posted!

Best, Marc

duke2906 commented 4 years ago

It may be worth to have a look on Kodein https://kodein.org/di/

timo-drick commented 4 years ago

Please do not introduce more complexity as necessary. It is much more difficult to understand the code when Dagger or similar frameworks are used. And it is also possible to do testing without this frameworks. Btw. in my opinion the testing argument is very weak to justify that the code will be obfuscated. Especially for this porject it is very important to keep every thing as simple as possible.

Eklow02 commented 4 years ago

If you want to introduce DI, I would suggest that you use koin (https://github.com/InsertKoinIO/koin). It doesn't create so much code like dagger and is easy to understand

svenjacobs commented 4 years ago

@timo-drick I don't see how the dependency injection pattern introduces "unnecessary" complexity? Actually you don't need a framework like Dagger 2 for implementation. The basic pattern is very simple and can be implemented "by hand". I see many benefits of this pattern like modularity, reusability and testability for example.

MrIceman commented 4 years ago

If you're such a fan ob singletons then I guess singleton factories would make it too, at the end this is what dagger2 generates. But dependency injection and testing is necessary for quality assurance, especially in a project like this. The DI framework at the end is just a detail, but you need to get rid of the singleton calls or you won't have testable code at all

AlexanderEggers commented 4 years ago

I am not part of this project, but I am happy to help with this part. I actually wrote my own annotation processor that simplifies the overall usage of Dagger2 for Android: https://github.com/Mordag/archknife Let me know if I can get start with switching to Dagger or my own lib.

yanniks commented 4 years ago

Google recently announced Dagger Hilt which should be the way to go I guess: https://developer.android.com/training/dependency-injection/hilt-android

I have implemented it in one of our projects already, with very high satisfaction.

MrIceman commented 4 years ago

there are many frameworks, just build your own factories imo. It's not that difficult

MovGP0 commented 4 years ago

there are many frameworks, just build your own factories

Even when implementing a DI framework is simple, I see no reason to reinvent the wheel. Using an existing DI framework is best practice.

d4rken commented 4 years ago

We've now introduced Dagger2 as DI framework in #1069 and will steadily increase DI integration in the project.

We picked dagger-android over hilt because there are only hilt -alpha releases available.