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

Add Koin instead of dagger-hilt as DI #49

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

santi-andracchi
Copy link

@santi-andracchi santi-andracchi commented Sep 22, 2021

Add Koin instead of Hilt as DI as an alternative for this base project

There are some pros and cons about it, at least that I can notice, such as:

PROS

  • We don´t have to use annotations, since Koin doesn´t generate any code
  • Lightweight apk
  • Less learning curve and boilerplate code
  • Smaller impact on our built time.

CONS

  • Dagger is a compile-time DI framework, we will know about our mistake almost instantly because our project will fail to build.

Here there are some interesting articles which explain the main differences between both:
Dagger-Hilt & Koin under the hood
Koin Vs Dagger-Hilt

PD: I also introduce a test library Mockk, which has very good stuff for testing coroutines, and supports Koin

Santiago Andracchi added 2 commits September 20, 2021 18:52
@@ -5,7 +5,7 @@
<option name="linkedExternalProjectsSettings">
<GradleProjectSettings>
<option name="delegatedBuild" value="false" />
<option name="testRunner" value="PLATFORM" />
<option name="testRunner" value="GRADLE" />
Copy link

@sfiamene sfiamene Feb 17, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to exclude files are already tracked but now are ignored through .gitignore you can do any of these steps:

  • git rm --cached <file> to exclude a file
  • git rm -r --cached <folder> to exclude a whole folder

package="com.package">

<application android:extractNativeLibs="true" />
</manifest>

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd add a new line here

Comment on lines +33 to +34
// viewModel { BottomBarViewModel(androidApplication()) }
// single { provideIotDao(androidContext()) }

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what do you think of removing these commented lines if they are no longer needed?

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

Successfully merging this pull request may close these issues.

3 participants