TODO-MVP: getTaskWithId could return dirty task

[mifkamaz]

Where is check mCacheIsDirty or why is it not necessary?

    @Nullable
    private Task getTaskWithId(@NonNull String id) {
        checkNotNull(id);
        if (mCachedTasks == null || mCachedTasks.isEmpty()) {
            return null;
        } else {
            return mCachedTasks.get(id);
        }
    }

[VaradGupta23]

What’s going on In TasksRepository, there’s a method:

@Nullable private Task getTaskWithId(@NonNull String id) { checkNotNull(id); if (mCachedTasks == null || mCachedTasks.isEmpty()) { return null; } else { return mCachedTasks.get(id); } } This method just returns whatever is in memory (mCachedTasks) without checking if the cache is marked as dirty (mCacheIsDirty).

The concern If mCacheIsDirty is true, it means:

The in-memory data might be outdated compared to the database or remote server.

Returning directly from mCachedTasks could give incorrect / stale task data.

Why it might be “okay” in the sample In the official sample:

getTaskWithId() is only called internally after a prior fetch that ensures fresh data.

mCacheIsDirty is always checked in public data-retrieval methods (e.g., getTasks()), and if dirty, the cache is refreshed before getTaskWithId() is used.

So, in the sample’s flow, mCacheIsDirty will be false whenever getTaskWithId() runs — making an explicit check unnecessary.

Safer alternative If you want to make the repository more robust and guard against future misuse:

@Nullable private Task getTaskWithId(@NonNull String id) { checkNotNull(id); if (mCachedTasks == null || mCachedTasks.isEmpty() || mCacheIsDirty) { return null; // Force caller to refresh } else { return mCachedTasks.get(id); } } Or even better: Force a fresh fetch if dirty:

@Nullable private Task getTaskWithId(@NonNull String id) { checkNotNull(id); if (mCacheIsDirty) { refreshTasks(); // Or trigger remote fetch } return (mCachedTasks != null) ? mCachedTasks.get(id) : null; } Conclusion for the issue Current sample doesn’t check mCacheIsDirty in getTaskWithId() because, by design, it’s only called after cache is confirmed fresh.

In real-world apps, adding a mCacheIsDirty check (or triggering a refresh) is safer to avoid accidental stale data access.