The Hidden Implication of Unnecessary Base Class

Today one of my colleagues came to ask me whether it is okay if she’s going to refactor some code in our project and remove some unnecessary classes. My first reaction is “Sure, why would you keep them if they are unnecessary?”

The project being refactored is just a simple Android app containing around 70 activities and fragments. The presentation layer is solely in MVVM pattern.

It’s so common in an Android app that many pages may display data in some kind of list. Generally, this will be implemented as RecyclerView under the hood. It is also possible that several types of items may be displayed in the same list.

With MVVM, we may choose to represent each item type in the list by different view model classes. Here is one page in the app, Reward History page, where it lists all appreciations (represented as “star” in the app) you’ve received and sent from/to your colleagues. Notice that it also shows “No stars sent today” item (RewardHistoryNoStarTodayHeaderItemModel) at a very first item if by far you haven’t sent any star today. Appreciations are also grouped by date which makes the list to show another type of item — the date header item (RewardHistoryGroupItemModel). Those items that display appreciation details like a number of stars, what reason and some additional thankful message you sent in any given appreciation are RewardHistoryItemModel.

Below is the package structure that keeps all those presentation classes.

rewardhistory/
 ├ adapters/
 │  └ RewardHistoryItemAdapter.kt
 ├ viewmodels/
 │  ├ RewardHistoryGroupItemModel.kt
 │  ├ RewardHistoryItemModel.kt
 │  ├ RewardHistoryItemModelBase.kt
 │  └ RewardHistoryNoStarTodayHeaderItemModel.kt
 ├ viewholders/
 │  ├ RewardHistoryGroupItemViewHolder.kt
 │  ├ RewardHistoryItemViewHolder.kt
 │  └ RewardHistoryNoRewardTodayHeaderViewHolder.kt
 ├ RewardHistoryActivity.kt
 └ RewardHistoryViewModel.kt

You can see that viewmodels/ folder keeps the class of each item. Now, my very colleague really hates RewardHistoryItemModelBase and would like to remove it because it is just a plain boring class defined as follows.

open class RewardHistoryItemModelBase : BaseObservable()

This is a base class for all 3 other item classes. It does nothing. It is just an empty base class which extends from BaseObservable (BaseObservable is a class from Android Data Binding API)

If our app does have only this page, the world would be simple and she will not totally be angry. But the fact that our app has 70+ activities/fragments in which ¼ of them has UI similar kind of this list. I mean this similar empty base class can be found a lot in our app!

She said she wants to wipe out all these empty, nonsense classes and change all item classes to directly extend from BaseObservable instead.

I was about to agree with her. Suddenly, my conscience asks me what happens if I want to know all possible classes of items on a particular page. If all items just extend from only the same BaseObservable, then there is no easy way to partition these. Yes, they may be inferred from project structure because we place all classes that relate to a particular page in the same package but its enforcement is very weak. New team members not familiar with the project are likely to put it in a wrong package. On the opposite, parent-child class hierarchy can’t lie us. You can always use Android Studio to list all derive classes from any given base class. So much stronger enforcement in this case.

That was my feeling and some lesson-learned I got from it. Sometimes, a class that looks like nothing turns out to be quite important. While it might not have any obvious responsibility at runtime, its sole purpose of being a “marker class” for the aspect of code documentation at development time is so promising.