Skip to content

Switch to dagger hilt#39

Open
vicky7230 wants to merge 1 commit intomasterfrom
feature/switch-to-hilt
Open

Switch to dagger hilt#39
vicky7230 wants to merge 1 commit intomasterfrom
feature/switch-to-hilt

Conversation

@vicky7230
Copy link
Owner

@vicky7230 vicky7230 commented Jul 30, 2025

PR Type

Enhancement


Description

  • Switched to Dagger Hilt for dependency injection

  • Updated navigation and ViewModel setup

  • Modified test configurations for Hilt


Diagram Walkthrough

flowchart LR
  A["Switch to Dagger Hilt"] -- "Update navigation" --> B["Modify ViewModel setup"]
  B -- "Adjust test configurations" --> C["Enhance testing with Hilt"]
Loading

File Walkthrough

Relevant files

@github-actions
Copy link

github-actions bot commented Jul 30, 2025

PR Reviewer Guide 🔍

(Review updated until commit 3ae18a9)

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪
🧪 PR contains tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Possible Issue

The setUp method no longer initializes notesDb, tagsDao, and notesDao. Ensure these are properly initialized using Hilt or another method.

hiltRule.inject()

@github-actions
Copy link

github-actions bot commented Jul 30, 2025

PR Code Suggestions ✨

Latest suggestions up to 3ae18a9
Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
General
Use libs catalog for dependencies

Use the libs catalog for dependency versions to maintain consistency and simplify
version management.

core/database/build.gradle.kts [49-51]

-androidTestImplementation("com.google.dagger:hilt-android-testing:2.56.2")
-// ...with Kotlin.
-kaptAndroidTest("com.google.dagger:hilt-android-compiler:2.56.2")
+androidTestImplementation(libs.hilt.android.testing)
+kaptAndroidTest(libs.hilt.compiler)
Suggestion importance[1-10]: 8

__

Why: Using the libs catalog for dependency versions helps maintain consistency and simplifies version management, which is important for project maintainability.

Medium
Add navigation methods

Consider adding functions to navigate between features for better encapsulation.

app/src/main/java/com/vicky7230/tasker2/navigation/DefaultNavigator.kt [1-4]

 data class DefaultNavigator(
     val notesFeature: NotesFeature,
     val addEditNoteFeature: AddEditNoteFeature,
     val tagsFeature: TagsFeature,
-)
+) {
+    fun navigateToNotes(navController: NavHostController) {
+        notesFeature.registerGraph(navController, this)
+    }
 
+    fun navigateToAddEditNote(navController: NavHostController) {
+        addEditNoteFeature.registerGraph(navController, this)
+    }
+
+    fun navigateToTags(navController: NavHostController) {
+        tagsFeature.registerGraph(navController, this)
+    }
+}
+
Suggestion importance[1-10]: 7

__

Why: Adding navigation methods can improve encapsulation and make the code more maintainable. However, it is not a critical change and does not address any existing issues.

Medium
Verify Hilt plugin configuration

Ensure that libs.plugins.hilt.android is correctly defined in your versions.toml or
dependencies.gradle.kts file.

app/build.gradle.kts [8-10]

+alias(libs.plugins.kotlinx.serialization)
+alias(libs.plugins.ksp)
+alias(libs.plugins.hilt.android)
 
-
Suggestion importance[1-10]: 6

__

Why: The suggestion asks to verify the configuration of the Hilt plugin, which is important but not critical. It does not involve code changes and is more of a verification step.

Low
Validate feature implementations

Ensure that NotesFeature, AddEditNoteFeature, and TagsFeature are properly
implemented and available.

app/src/main/java/com/vicky7230/tasker2/navigation/DefaultNavigator.kt [1-4]

+data class DefaultNavigator(
+    val notesFeature: NotesFeature,
+    val addEditNoteFeature: AddEditNoteFeature,
+    val tagsFeature: TagsFeature,
+)
 
-
Suggestion importance[1-10]: 6

__

Why: Ensuring that the features are properly implemented is important for the functionality of the application. However, this suggestion does not involve code changes and is more of a verification step.

Low
Add documentation to interface

Ensure that the Feature interface is properly documented and that all implementing
classes adhere to the expected contract.

core/common/src/main/java/com/core/common/navigation/Feature.kt [6-10]

+/**
+ * Interface for registering feature-specific navigation graphs.
+ */
 interface Feature {
+    /**
+     * Registers a navigation graph with the provided [NavHostController] and [NavGraphBuilder].
+     *
+     * @param navHostController The [NavHostController] to use for navigation.
+     * @param navGraphBuilder The [NavGraphBuilder] to build the navigation graph.
+     */
     fun registerGraph(
         navHostController: NavHostController,
         navGraphBuilder: NavGraphBuilder,
     )
Suggestion importance[1-10]: 6

__

Why: Adding documentation improves code readability and maintainability, but it does not address any critical functionality or bug.

Low
Possible issue
Initialize Hilt dependencies correctly

Ensure that hiltRule.inject() is called after setUp() to properly initialize Hilt
dependencies.

core/database/src/androidTest/java/com/core/database/TagsDaoTest.kt [30]

-hiltRule.inject()
+@Before
+fun setUp() {
+    hiltRule.inject()
+    notesDb = hiltRule.getApplicationContext<Application>().applicationComponentTest.notesDb()
+    tagsDao = notesDb.getTagsDao()
+    notesDao = notesDb.getNotesDao()
+}
Suggestion importance[1-10]: 7

__

Why: The suggestion is correct but only asks to verify or ensure a change done in the PR, which should not receive a score above 7. The hiltRule.inject() call is already in the correct place, and the additional lines in improved_code are not part of the PR.

Medium

Previous suggestions

Suggestions up to commit aae5afe
CategorySuggestion                                                                                                                                    Impact
Security
Use parameterized queries

Use parameterized queries to prevent SQL injection and improve code readability.

core/database/src/main/java/com/core/database/di/DatabaseModule.kt [39-42]

 db.execSQL(
-    "INSERT INTO notes (content, tagId, timestamp, date, time) VALUES ('Welcome to your notes app! This is your first note.','1','" +
-        System.currentTimeMillis() +
-        "','" +
-        LocalDate
-            .now()
-            .format(DateTimeFormatter.ISO_DATE) + "','00:00:00')",
+    "INSERT INTO notes (content, tagId, timestamp, date, time) VALUES (?, ?, ?, ?, ?)",
+    arrayOf(
+        "Welcome to your notes app! This is your first note.",
+        "1",
+        System.currentTimeMillis().toString(),
+        LocalDate.now().format(DateTimeFormatter.ISO_DATE),
+        "00:00:00"
+    )
 )
Suggestion importance[1-10]: 8

__

Why: Using parameterized queries prevents SQL injection and improves code readability, which is a significant security and maintainability improvement.

Medium
General
Annotate module for Hilt

Annotate the module with @InstallIn(SingletonComponent::class) to ensure it is
installed in the SingletonComponent.

core/database/src/main/java/com/core/database/di/DatabaseModule.kt [21]

+@InstallIn(SingletonComponent::class)
 @Module
 class DatabaseModule {
Suggestion importance[1-10]: 6

__

Why: Annotating the module with @InstallIn(SingletonComponent::class) is important for Hilt, but it's not a critical change.

Low
Verify module installation

Ensure that the NetworkModule is correctly annotated with
@InstallIn(SingletonComponent::class) and that all dependencies are properly
provided using Hilt.

core/network/src/main/java/com/core/network/di/NetworkModule.kt [9-10]

+@InstallIn(SingletonComponent::class)
 @Module
 class NetworkModule {
     @Singleton
Suggestion importance[1-10]: 6

__

Why: The suggestion asks to verify the module installation, which is a good practice, but it does not address any critical issue or bug.

Low
Validate binding correctness

Confirm that AddEditNoteFeature and AddEditNoteFeatureImpl are correctly defined and
that the binding is accurate.

feature/add_edit_note/ui/src/main/java/com/feature/add_edit_note/ui/di/AddEditNoteUiModule.kt [14]

+@Binds
+abstract fun bindAddEditNoteUi(addEditNoteApiImpl: AddEditNoteFeatureImpl): AddEditNoteFeature
 
-
Suggestion importance[1-10]: 6

__

Why: The suggestion asks to validate the binding correctness, which is a good practice, but it does not address any critical issue or bug.

Low
Verify Hilt plugin definition

Ensure that libs.plugins.hilt.android is correctly defined in your
libs.versions.toml or equivalent configuration file.

app/build.gradle.kts [8-10]

+alias(libs.plugins.kotlinx.serialization)
+alias(libs.plugins.ksp)
+alias(libs.plugins.hilt.android)
 
-
Suggestion importance[1-10]: 5

__

Why: The suggestion asks to verify the definition of libs.plugins.hilt.android, which is a good practice but does not directly improve the code. It should not receive a high score as it does not address a critical issue or provide a significant improvement.

Low
Validate Hilt injection

Ensure that DefaultNavigator is properly provided by Hilt and that all required
dependencies are correctly annotated.

app/src/main/java/com/vicky7230/tasker2/MainActivity.kt [30-33]

+@AndroidEntryPoint
+class MainActivity : ComponentActivity() {
+    @Inject
+    lateinit var defaultNavigator: DefaultNavigator
 
-
Suggestion importance[1-10]: 5

__

Why: The suggestion asks to verify the Hilt injection, which is important for ensuring the correctness of the dependency injection setup. However, it does not provide a direct code change and should not receive a high score.

Low
Validate graph registration

Ensure that registerGraph methods are correctly implemented and that SubGraph.Notes
is a valid destination.

app/src/main/java/com/vicky7230/tasker2/navigation/App.kt [72-85]

+NavHost(navController = navHostController, startDestination = SubGraph.Notes) {
+    defaultNavigator.notesFeature.registerGraph(
+        navHostController = navHostController,
+        navGraphBuilder = this
+    )
+    defaultNavigator.addEditNoteFeature.registerGraph(
+        navHostController = navHostController,
+        navGraphBuilder = this
+    )
+    defaultNavigator.tagsFeature.registerGraph(
+        navHostController = navHostController,
+        navGraphBuilder = this
+    )
+}
 
-
Suggestion importance[1-10]: 5

__

Why: The suggestion asks to verify the implementation of registerGraph methods and the validity of SubGraph.Notes. While important for correctness, it does not provide a direct code change and should not receive a high score.

Low

@vicky7230 vicky7230 force-pushed the feature/switch-to-hilt branch from aae5afe to 3ae18a9 Compare July 30, 2025 06:32
@github-actions
Copy link

Persistent review updated to latest commit 3ae18a9

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant