ViewModel method name convention

Question

What is the best way to name methods in the ViewModel classes? Based on its action/behavior or the lifecycle of the Activity/Fragment?

For example:

Methods named by its action

override fun onResume() {
    super.onResume()
    viewModel.connect()
}

override fun onPause() {
    super.onPause()
    viewModel.disconnect()
}

override fun onItemCheckedChanged(task: Task, value: Boolean) =
    viewModel.updateTaskStatus(task, value)

Methods named by Android lifecycle

override fun onResume() {
    super.onResume()
    viewModel.onResume()
}

override fun onPause() {
    super.onPause()
    viewModel.onPause()
}

override fun onItemCheckedChanged(task: Task, value: Boolean) =
    viewModel.onItemCheckedChanged(task, value)

There are several examples in the internet and the two approaches are used.

In my opinion, the methods should be related to the lifecycle, in this way the View does not need to know the logic behind the ViewModel, it just need to know that a lifecycle method need to be called.

What is the best approach?

Answer 1

There is not a correct way, as long as the code is clean and easy to read/understand. But if you look at the examples Android give, they show methods similar to the ones you posted.

1) One of the ways is to have an object with the methods named by Android lifecycle (Which you mentioned).

class MyLocationListener {
    public MyLocationListener(Context context, Callback callback) {
        // ...
    }

    void start() {
        // connect to system location service
    }

    void stop() {
        // disconnect from system location service
    }
}

Each function is manually called within the lifecycle owner like so:

@Override
public void onStart() {
    super.onStart();
    myLocationListener.start();
    // manage other components that need to respond
    // to the activity lifecycle
}

@Override
public void onStop() {
    super.onStop();
    myLocationListener.stop();
    // manage other components that need to respond
    // to the activity lifecycle
}

2) However, if you want to name the methods by their actions, you could accompany them methods with the OnLifecycleEvent annotation, which was excitingly brought to us in Android Jetpack! So for example:

public class MyLocationListener implements LifecycleObserver {
    @OnLifecycleEvent(Lifecycle.Event.ON_RESUME)
    public void connectListener() {
        ...
    }

    @OnLifecycleEvent(Lifecycle.Event.ON_PAUSE)
    public void disconnectListener() {
        ...
    }
}

Now these methods are called automatically with the help of a LifecycleObserver which can observe a LifecycleOwner:

myLifecycleOwner.getLifecycle().addObserver(new MyLocationListener());

The LifecycleOwner is typically an Activity or Fragment. Up to you which one you choose, however my preferred is the LifecycleObserver as it requires less code, which I think makes it look cleaner.

If your interested in good Android practices and some tips to help you along the way, there are a few good pages I would recommend:

- Android best practices

- Android tips & tricks

- Android must have libraries

Answer 2

IMO the best approach is to use the name of the lifecycle for these reasons:

1. It makes the view (fragment) dumber. In effect, as soon as you name the function connect() the view is making the decision (logic) to connect.
2. What happens if now you want to do something else in onResume(), like refresh data? Do you add another function called refreshData() and call it after connect()? Or do you call it before connect() because you know that the data must be refreshed before you can connect. Oops you've added more implicit logic to your view, that a test of just the viewmodel will fail to catch. Better to just call fun onResume() { refreshData(); connect(); }
3. In general you should name all your view events along the lines of "something happened to me". Eg. didPressDowload() orderConfirmed() etc. If you name it like orderPizza() instead, that's like a command and if you want to fail the order like fun orderPizza() { if (orderEmpty) return ....} then that function no longer is doing what it says in the name. It's small semantics sometimes, but other times it really saves you errors and getting confused about how to keep logic in the viewmodel only. See https://redux.js.org/style-guide/style-guide#model-actions-as-events-not-setters for similar advice from the web world.