What I Look For in an Android Code Review

I've reviewed thousands of pull requests. Early in my career, I focused on style: naming conventions, formatting, whether the code "looked clean." That's the least important part of a code review.

Now I focus on things that matter in production. Things that cause bugs, performance regressions, and maintenance headaches months later. Here's my checklist.

1. Lifecycle Awareness

The number one source of Android bugs is lifecycle mismanagement. I look for:

Coroutines that outlive their scope. A coroutine launched in viewModelScope is fine. A coroutine launched in GlobalScope or a raw CoroutineScope without proper cancellation is a leak waiting to happen.

// Red flag: no cancellation management
class MyViewModel : ViewModel() {
    fun loadData() {
        GlobalScope.launch { // Will outlive the ViewModel
            val data = repository.fetch()
            _state.value = data
        }
    }
}
 
// Correct: scoped to ViewModel lifecycle
class MyViewModel : ViewModel() {
    fun loadData() {
        viewModelScope.launch {
            val data = repository.fetch()
            _state.value = data
        }
    }
}

State collection without lifecycle awareness. In Compose, collectAsStateWithLifecycle() respects the lifecycle. collectAsState() doesn't. That one word difference can mean the difference between a well-behaved app and one that processes updates while in the background.

2. Threading and Concurrency

Main thread work. Any disk I/O, network call, or heavy computation on the main thread is an immediate blocker. I search for Room calls without suspend, SharedPreferences reads in UI code, and JSON parsing in adapters.

Race conditions. When two coroutines can modify the same state, I look for proper synchronization. Mutex, StateFlow, or Channel are fine. Bare var with no protection is not.

3. Error Handling

Swallowed exceptions. An empty catch block is almost always wrong. Even if you can't recover, log it.

// Red flag
try {
    api.submitData(payload)
} catch (e: Exception) {
    // silently swallowed
}
 
// Better
try {
    api.submitData(payload)
} catch (e: IOException) {
    logger.warn("Submit failed, will retry", e)
    retryQueue.enqueue(payload)
} catch (e: Exception) {
    logger.error("Unexpected error during submit", e)
    _state.value = UiState.Error("Something went wrong")
}

Catching too broadly. catch (e: Exception) hides programming errors (NPEs, ClassCastExceptions) behind the same handler as expected failures (IOExceptions). I look for specific exception types.

4. Data Flow

One-way data flow. State should flow down (ViewModel to UI) and events should flow up (UI to ViewModel). When I see UI components directly modifying shared state or calling repository methods, that's a red flag.

Single source of truth. If the same data is stored in two places (ViewModel state and a separate cache, for example), they will eventually diverge. I look for one authoritative source with everything else derived from it.

5. Performance Signals

Unnecessary recompositions. In Compose, I look for unstable parameters (lists, lambdas created in composition) that cause recomposition of expensive composables. A remember or derivedStateOf is often the fix.

Over-fetching. Loading all items when only the first page is needed. Fetching a full object when only one field is used. These are easy to miss in review but compound at scale.

Bitmap handling. Any code that loads images without size constraints is a potential OOM. I verify that image loading uses proper downsampling and caching.

6. What I Don't Focus On

Style preferences. If the code is consistent with the codebase's existing style, I don't comment on formatting choices.

Minor naming opinions. If the name is clear and accurate, I don't suggest alternatives just because I would have named it differently.

"I would have done it differently." This isn't a reason to block a PR. Unless the alternative is meaningfully better in a concrete way (performance, readability, correctness), different isn't better.

The Meta-Skill

The best code reviewers I know share a trait: they review the change in the context of the system, not just in isolation. A function that looks fine by itself might be a problem because it duplicates logic that exists elsewhere, or because it introduces a new pattern that conflicts with the team's conventions.

Code review is a conversation, not an audit. The goal is a better codebase, not a perfect PR.