architecture-samples icon indicating copy to clipboard operation
architecture-samples copied to clipboard

[master] method never used: TasksViewModel.addNewTask()

Open BeQuietLee opened this issue 6 years ago • 1 comments

The method addNewTask() in TasksViewModel.kt is never used. Though commented "Called by the Data Binding library and the FAB's click listener." The click listener of FAB is set in TasksFragment.kt, navigating to new page.

TasksViewModel.kt

    /**
     * Called by the Data Binding library and the FAB's click listener.
     */
    fun addNewTask() {
        _newTaskEvent.value = Event(Unit)
    }

TasksFragment.kt

    private fun setupFab() {
        activity?.findViewById<FloatingActionButton>(R.id.add_task_fab)?.let {
            it.setOnClickListener {
                navigateToAddNewTask()
            }
        }
    }

    private fun navigateToAddNewTask() {
        val action = TasksFragmentDirections
            .actionTasksFragmentToAddEditTaskFragment(
                null,
                resources.getString(R.string.add_task)
            )
        findNavController().navigate(action)
    }

BeQuietLee avatar Sep 18 '19 03:09 BeQuietLee

You're right — the TasksViewModel.addNewTask() method is declared but never used, even though the comment claims:

"Called by the Data Binding library and the FAB's click listener."

But as of the current implementation in TasksFragment.kt, the FAB is directly calling navigateToAddNewTask(), bypassing the ViewModel entirely.

Breakdown TasksViewModel.kt

fun addNewTask() { _newTaskEvent.value = Event(Unit) } This function emits an event via LiveData (_newTaskEvent) — presumably meant for the UI to react (e.g., navigation).

However, no observer or data binding is actually calling it anymore.

TasksFragment.kt

it.setOnClickListener { navigateToAddNewTask() } Directly calls the navigateToAddNewTask() function on FAB click.

Solution Options Option 1: Remove the unused addNewTask() method If you're not using LiveData<Event> anymore for navigation and are handling all routing via Fragment → NavController, this method is dead code and should be deleted.

// DELETE this if not using data binding or event observers fun addNewTask() { _newTaskEvent.value = Event(Unit) } Option 2: Re-enable ViewModel-based navigation If you prefer separation of concerns, and want the Fragment to observe the ViewModel for navigation:

Modify the FAB click to call the ViewModel:

it.setOnClickListener { viewModel.addNewTask() } Then, in TasksFragment, observe newTaskEvent:

viewModel.newTaskEvent.observe(viewLifecycleOwner) { event -> event.getContentIfNotHandled()?.let { navigateToAddNewTask() } } This uses SingleLiveEvent/Event wrapper to ensure one-time navigation. Recommendation For clean architecture, Option 2 is better:

Keeps Fragment UI-only

Delegates logic (even navigation triggers) to the ViewModel

Easier to unit test ViewModel behavior

But if you're following a simplified approach, Option 1 (deletion) is fine.

VaradGupta23 avatar Jul 21 '25 08:07 VaradGupta23