Wikimedia Apps/Team/Android/Development philosophy

This document outlines the general philosophy of our development practices in the Android team, and a few high-level guidelines for effective and successful Android development for the present and the future.

General philosophy
Because the size of our user base is much smaller than mobile web, yet still large enough to have an impact for many crucial audiences, we have a unique opportunity to be much more experimental in the types of functionality we provide. Our team can be (and has been!) an incubator for new ideas, to be piloted on our platform, and then either adopted on other platforms (e.g. mobile web), or retired quickly, with feedback gathered and lessons learned.

This means that we need to be able to move unusually quickly in our development cycle. That’s why the things I’m going to say might sound heretical to a lot of other engineers. The first thing of this sort is: dogmatic adherence to conventions (e.g. strict style guidelines, percent-targets for test coverage, goals for lint-free code, etc.) can easily become a hindrance to this philosophy, instead of a benefit.

How strict is too strict?
Let's get real for a minute. We are not building firmware for nuclear centrifuges. Neither are we building an air traffic control interface, nor are we building image processing for MRI machines. We are building an Android app for reading Wikipedia articles.

Strict adherence to coding conventions, test coverage, etc. should be proportional to how far upstream you are. If you're submitting a patch to the Linux kernel, then it makes sense for every character of your code to be scrutinized as much as possible. If, however, you're building a new feature for a mobile app that displays Wikipedia articles, the rules can surely be relaxed somewhat.

We are not writing code that will need to be maintained by future generations of developers twenty years from now. Everything about our platform is ephemeral. New languages will spring up, new frameworks and libraries will be adopted, new designers will redesign the entire interface, and the entire platform can one day disappear from under our feet, to be supplanted by a new and better platform. And all of that is OK!

We're by no means advocating for an abandonment of best practices and norms, but rather for a better balance of reasonable guidelines and an awareness that our product is a perpetual work in progress with an acceptable failure rate.

We are all product owners
Our team has a dedicated official product owner, but guess what: we are all product owners. The product belongs to all of us, and we are responsible for it, and accountable to our users. To that extent, we should all be dogfooding our product, i.e. using it in our daily lives. If the product is not for us, then who is it for?

We should be quietly but persistently encouraging our friends and family to use it, asking what features they would like to see, and getting feedback about their experiences with it. If the product is not for them, then who is it for?

If you’re finding that you’re not using our app in your daily life, ask yourself "Why not?" What potential feature in the app would make you want to use it? What feature would you be particularly excited about building? And... why haven’t you built it yet?

Code review
Reviewing someone else’s code should consist of two questions:
 * Does it work as expected and (if there’s a user-interface component) look as designed?
 * Is the code semi-competent?

The next few sections will explore what is meant by “competent” code. But first, a few words on the code review process itself.

The comments you leave when reviewing code should be as positive and constructive as possible. Engineers have a natural tendency towards defensiveness when something they’ve created gets criticized or otherwise “reviewed” by someone else. Since code reviews are restricted to plain text, it makes them vulnerable to a loss-in-translation effect where any nuance, intonation, or emotion of the reviewer is lost.

Therefore one must be especially mindful of maintaining positivity when leaving comments in code reviews. Put yourself in the position of the reviewee, and consider how you would feel if you received the comments you’re about to give. Try to formulate your comments as questions instead of statements. For example, instead of saying “This block of code is utter nonsense and should be rewritten,” try saying “Would it be better if we restructure this code like this: ___?”

Conversely, you should never hesitate to request changes on a patch if something doesn’t look right. And you also shouldn’t expect a patch of yours to be merged on the first pass, without one or two follow-ups.

And once again, when reviewing code, keep in mind all of the philosophies outlined previously (rapid development and all that), and try to limit your insistence on code style perfection, especially if the code is working properly, or if the argument over style is getting in the way of real development of the next meaningful feature.

Focus on performance
One thing on which we must not compromise is performance (i.e. speed and responsiveness). Let’s not forget that one of the great benefits of native apps is better performance than can be achieved in mobile web. If we let our focus on performance slip, we would be defeating one of our raisons d’être.

Free up the UI thread
Do not write code that does too much work on the main thread. The Android OS helpfully alerts you in `logcat` when this is happening. We have several classes and constructs to perform asynchronous operations easily, so make sure to use them.

Of course, when an asynchronous operation returns to the main thread, make sure to check the state of the current Activity or Fragment (i.e. make sure it’s still active or attached to an active Context).

Minimize and optimize database interactions
Our app uses a SQLite database for storing a lot of things including reading lists, browsing history, thumbnail URL caches, etc. Interactions with this database must be as efficient as possible, so that the user interface remains responsive and seamless (see previous point). The way to do this is to be aware of how SQLite works and how to optimize it. To name a couple of specific examples: Don’t perform multiple queries in a row, when a single better-crafted query will suffice. When it’s necessary to perform multiple queries, or to perform queries in a loop, wrap them with beginTransaction and endTransaction calls.

Don’t overcrowd View hierarchies
Views that are displayed in an Activity are actually very expensive objects, and should be used sparingly. When there are too many Views on the screen, it can slow down the rendering process significantly. The problem is compounded when we have a complex View hierarchy inside RecyclerView items (i.e. Views that are repeated numerous times, or nested very deeply). Therefore, keep the following guidelines in mind: When possible, use the newly-introduced ConstraintLayout, which provides a great deal of flexibility for child Views, and can effectively flatten View hierarchies that would otherwise be badly nested. When using a RecyclerView, make sure it’s used properly: specifically, make sure that its height is set to match_parent, and never wrap_content, so that the RecyclerView doesn’t accidentally render all of its children, including ones that are off-screen and don’t need to be rendered in the current window.

Don’t use too much memory
Our platform has fairly limited resources, especially when compared to the nearly-infinite resources of desktop platforms. Therefore we need to be mindful of how much VM memory we allocate. Android Studio has a helpful real-time tracker of memory usage when debugging the app, which can be useful for tracking down memory hogs.

Don’t marshal/unmarshal huge objects unnecessarily
Transforming a plain Java object to its JSON representation (in a String) is a pretty expensive operation, and should be done only when necessary. Other more specific points: Don’t send huge amounts of data across the WebView Javascript Bridge. This is also an expensive operation, and should be restricted to modest amounts of data. Don’t exchange huge amounts of data between Activities. That is, don’t marshal huge objects into a Bundle and sent it to an Activity inside an Intent. The Android OS actually imposes a hard limit on the maximum size of data that you can send this way, and will crash the app if this size is exceeded.

Keep an eye on APK size and method count
At the time of this writing, our method count is approaching 50000. While this is still far from the danger zone, we still need to be mindful of eventually reaching the dreaded dex limit of 65535 methods. Exceeding this limit will be a significant hit to our performance, and should be avoided if possible.

Our current APK size (again, at the time of this writing) is around 23 MB, which is actually relatively small compared to most other major apps. Still, it’s worth keeping an eye on how our APK size is impacted by new features that we add, or new libraries that we depend on.

Unit tests
Ever a source of heated debate among developers, here is my take on unit testing: There are tests that are extremely useful and helpful, and there are also tests that are unnecessary, redundant, and generally valueless. Distinguishing between the two is, as everything else, a fine balance.

Here are some examples of good unit tests:


 * Our Translation tests (i.e. the tests that verify the formatting of all of our localized strings) are vitally useful, and routinely catch errors that would otherwise lead to incorrect presentation of text in various languages.
 * Tests having to do with handling and transforming URIs, Dates, PageTitles, and WikiSites. These objects are of fundamental importance to nearly all features in the app, and should therefore be tested exhaustively, especially when dealing with edge cases such as language variants, URL fragments, non-main namespaces, etc.

And here are some examples of questionable unit tests:


 * Screenshot tests. The technology for staging screenshot tests is not yet mature, and requires enormous effort to set up and maintain. Furthermore, the human eye is a perfectly good screenshot test, and since all of us should be using our product in our daily lives (do we not?), this form of screenshot test should ideally be running in real time, for free.
 * Most of our network client and callback tests. Our network tests consist of feeding a canned response into a network client, and observing whether the correct callbacks are called. These tests are useful only if the format of the network response never changes, which in practice is not guaranteed. And once again, the unit test of the human eye can often pick up regressions when a network client fails.

So once again, the best unit tests are ones that test objects that are upstream, and objects that are used in many different contexts. Whereas bad unit tests are ones that test functionality that is high-level enough to be caught by routine usage of the product. Even worse tests are ones that require regular maintenance, for no other reason than to keep them passing, i.e. tests that get in the way of development.