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.

Alright, let’s finally dive in to the actual practical guidelines for development of our product, from the specific perspective of Android engineering.

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  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  or   (i.e. make sure it's still active or attached to an active  ).

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  and   calls.

Don't overcrowd View hierarchies
Views that are displayed in an Activity are actually rather 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  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, 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, and never  , 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.
 * When creating a custom View, make sure that the root node in your custom XML layout is a  node (with   attributes to aid laying out the view in the Android Studio designer), instead of an unnecessary root FrameLayout or something similar.

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 a fairly 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  and sent it to an Activity inside an  . 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 about 45000. 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, or 10 MB when split into separate ABIs, 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.

Don't create memory leaks
Yes, if you try hard enough, you can create memory leaks even in a garbage-collected language like Java. This can be done by storing references to an Activity or Context in a  variable, among other unfortunate examples.

Write clean code
The code that you write should be clean and understandable. With a little practice, this can become second nature.

Naming
Be generously descriptive when naming your variables, classes, and functions. There is no performance cost in having verbose names. Providing descriptive names will be a benefit to everyone including yourself, the next time you have to look at your own code.
 * Give descriptive names to classes, fields, function parameters, even local variables. You'll thank yourself later. The only permissible non-descriptive variable is, when used as the index in an innermost loop.
 * Include units of measurement into the name of your variables and constants. Suppose you have a constant called . What are the units of that constant? It might be obvious to you that the units are milliseconds, but it might not be obvious to everyone else, so a better name for the constant would be.
 * Give descriptive names to resources, such as strings and View identifiers. If you have a Button in your Activity, don't call it, but instead give it a name that actually reflects what it does, for example.
 * A function should be named after the thing that it does. Avoid function names like  or   or , unless you're entering a code obfuscation contest.
 * Member variables no longer need to start with the letter "m", or use any other kind of Hungarian notation, because it's 2018 and we have modern IDEs that provide different highlighting to reflect the type of variable you're looking at. mOK?

Classes and methods

 * A class should have a single responsibility.
 * Ideally, all of the dependencies of the class should be stated in the constructor.

Annotations
The Support Library provides numerous helpful annotations that can make our code more comprehensible, as well as instruct the IDE to alert us to possible issues. Get into the habit of including annotations whenever they are applicable, specifically:


 * If the return value from a method is not a primitive type, it should be annotated as  or , to make it explicitly clear what the method returns. The same applies to annotating parameters that are passed to methods, to make it explicitly clear what the method expects to receive. Generally we should lean towards enforcing   wherever possible.
 * If a parameter is an integer type, but actually refers to a resource identifier, you should use the appropriate  annotation, such as ,   and  ,  , and so on.
 * If you're building a class in which you expect descendant classes to call the  method for overridden methods, then use the   annotation.

Modern language features

 * Make use of the latest syntactic sugar provided by the language, such as lambdas. However, prior to using such features, do a little research on how they actually unfold under the hood, and what possible performance cost they might incur.

ButterKnife

 * When creating a new Activity or custom View, use ButterKnife for binding the Java code to the layout resources (using the  annotation). Ideally we should never have to call   ourselves.  Whenever possible, also use the convenient   and other similar annotations.

Colors and styles

 * Avoid using raw color values in XML layouts and in code. Instead use  resources, or better yet,  ibute resources that point to the corresponding color in the current theme.

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.