Improving the engineer experience

I’m yhanada from the Merpay Android Team, and I’m responsible for day 16 of Merpay Advent Calendar 2020.

I’ve worked as a Mercari/Merpay Android engineer for four years as of this December. In addition to my normal team role developing features, I’m involved in a project called Developer EXperience (DEX) Improvement, where I work on reducing the workload of engineers, such as by performing refactoring, considering rules, and removing unnecessary tasks.

In other words, DEX Improvement includes not only refactoring and improving code, but also organizational engineering (such as negotiating and coordinating with other teams).

In this article, I talk about some of the DEX improvements I’ve worked on, and then pick up from a previous blog article ("Implementing screen transitions in a multi-module project") to tackle an unresolved problem discovered there: reducing the complexity of screen transition definitions extending over modules in a multi-module project.

Transitioning between screens split with multi-modules

As mentioned in the previous article, Merpay Android features are designed using multi-modules. This means that, for example, if we want to define the transition from the feature A screen in the :feature_a module to the feature B screen in the :feature_b module, we cannot do so in either module.
If we look at the dependence relationship between these modules, it seems best to define screen transitions in the :app module.

Multi-module interdependence structure

I’ll be using some simplified code throughout this article. Here are the major components used in this article and their roles.

  • Controller: The component used to implement screens. Provided by Conductor.
  • FeatureXxxController: The class used to implement screens for a particular feature.
  • Navigation: The shared interface used to define screen transitions.
  • NavigationRegistry: Controls the implementation of Navigation components for each screen.
  • FeatureXxxNavigation: The interface used to define transitions for each screen (FeatureXxxController). Inherits Navigation.
  • FeatureXxxNavigator: The class for implementing the FeatureXxxNavigation interface.

Until just recently, the Navigation definition and implementation class for each screen were written as follows.

// FeatureAControllerの遷移処理定義
interface FeatureANavigation : Navigation {
    fun navigateToFeatureB(router Router)
}

// FeatureANavigationの実装クラス
class FeatureANavigator : FeatureANavigation {
    override fun navigateToFeatureB(router Router) {
        // …..
    }
}

FeatureANavigator is the implementation class of Navigation. When adding features, they would be added as FeatureA, FeatureB, etc. The new Navigators would be registered individually in NavigationRegistry by the engineer responsible for implementation.

class App : Application() {
    override fun onCreate() {
        super.onCreate()
        NavigationRegistry(
            FeatureANavigator(),
            FeatureBNavigator(),
            // 以下数十個のNavigator...
        )
    }
}

Problems discovered as the number of screens to register increased

Wecontinued to use this method for development for a while, but the team gradually discovered three major problems with this.

1) There was a tendency to forget to register Navigator instances in NavigationRegistry.
2) When registering a Navigator, many engineers added lines in the same area, making it easy for conflicts to occur (also, because conflicts must be fixed manually, sometimes required registrations got lost in the confusion).
3) We didn’t know what to do about Navigators for screens to use only with debug builds.

The process up to creating Navigators was fine, but engineers tended to forget to register them in NavigationRegistry, so we ran into problem 1 often at first. However, this occurred less often as we got used to the multi-module structure.

As we continued to get used to this, we started seeing problem 2 more often. We have around 10 Android engineers at Merpay, and depending on the period may have up to 10 projects to work on. That means we are always developing multiple features in parallel, which frequently results in conflicts in files that are edited by nearly everyone (such as NavigationRegistry). If we mess up when trying to resolve a conflict, we run into problem 2, where a Navigator registered by someone else is removed at some point. A missing registration doesn’t cause a build error, so we sometimes wouldn’t realize what happened until we tried running the application.

Personally speaking, I feel that the last problem listed above is the most important. Some screens are used only with the debug version, and so there are cases where we want to register a Navigator with only the debug version. That’s problem 3.

Navigators required by BuildType

Solution 1: Define an array to return Navigator instances for each BuildType

When developing an Android application, the typical way we implement screens that will be used only with the debug build is to provide classes with the same names in the folders for each BuildType, and then to reference this from the App class.

  • app/src/debug/java/com/merpay/navigation/NavigatorProvider.kt
  • app/src/release/java/com/merpay/navigation/NavigatorProvider.kt
object NavigatorProvider {
    val navigators: Array<Navigation> = arrayOf(
        FeatureANavigator(),
        FeatureBNavigator(),
        // debugフォルダ以下に置かれている方では
        // 例えばDebugNavigator()が追加されている
    )
}

Although this solves problem 3, this makes problems 1 and 2 even more complicated. It would be very bad if someone forgot to register a Navigator in the release build!

Solution 2: Create an annotation processor to extract required Navigators

For Merpay, we were able to resolve problem 3 using the BuildTypes introduced in solution 1. However, we took a different approach to resolve this issue with Mercari, which is implemented using the same multi-module configuration. This is because a different architecture is used to configure screens for Mercari and Merpay, and so the same approach cannot be taken.

For Mercari, we use annotations to associate transition callers and call targets, in order to check whether the specification of a transition target across modules is defined correctly. If the association is broken on one side, it will cause a build error.

For Merpay, we initially had attempted to introduce a mechanism to use annotations to check associations, but it would have ultimately required making modifications in two areas, rendering it mostly meaningless. We then came up with a new plan to instead create an annotation processor that would automatically extract a list of Navigators with annotations.

This added annotation is defined as follows.

@Retention(AnnotationRetention.SOURCE)
@Target(AnnotationTarget.CLASS)
annotation class ControllerNavigation

This annotation is used as a flag for extraction when building, so there’s no need to save it in the class file. AnnotationRetention.SOURCE is specified as the retention value.

The target for this annotation is the Navigation implementation class only, and AnnotationTarget.CLASS is specified as the target.

The following is a broad overview of what we want to accomplish with the annotation processor.

  1. Find classes with the @ControllerNavigation annotation,
  2. take an array of FQCNs of these classes,
  3. And generate a file defining these as object vals.

We used KotlinPoet to automatically generate files.

So what’s the difference when we add this annotation?
The engineer only has to directly edit the portion up to the Navigator class. The annotation is added as follows.

@ControllerNavigation // <- Added
class FeatureANavigator : FeatureANavigation {
    override fun navigateToFeatureB(router Router) {
        // .....
    }
}

The following type of object is generated by the annotation processor.

object MerpayNavigators {
    val navigators: Array<Navigation> =
arrayOf(
    com.merpay.featureA.FeatureANavigator(),
    com.merpay.featureB.FeatureBNavigator(),
    // ......
    )
}

The App class simply references the Navigator list provided by this object, and there is no need for the engineer to make any direct changes.

class App : Application() {
    override fun onCreate() {
        super.onCreate()
        NavigationRegistry(*MerpayNavigators.navigators) // Very clean!
    }

    // ......
}

Conclusion

When working on the DEX Improvement project, we have to consider various improvements and problems outside of feature development. If we discover a problem that cannot be resolved only for Merpay and that could also impact Mercari, we bring it up with the Client Platform Group (CPG) and work on resolving it together.

A dedicated member of the QA Team is also assigned to the DEX Improvement Team, and we can establish a release plan separate from that of other feature development.

You may think you’ve written some really clean code, but with time it will become obsolete (even if you weren’t rushed when coding due to lack of time). I work on the DEX Improvement project as I continue to rewrite code, with the goal of increasing engineer motivation and product standards.

In order to accomplish the goal introduced in this article (registering Navigators), solution 1 probably would be enough as long as you can catch complicated issues like problems 1 and 2.

Generally speaking, DEX improvement projects are the work of volunteers thinking every day about how to make things even better (as shown by solution 2). I believe that these efforts are taken to systematically resolve minor problems, ideas, etc. brought up by individuals.

Merpay is hiring Android engineers who share a belief in our mission and values. If that’s you, we’d love to work with you!

  • X
  • Facebook
  • linkedin
  • このエントリーをはてなブックマークに追加