architecture-samples icon indicating copy to clipboard operation
architecture-samples copied to clipboard

Why not the same as deleteAllTasks()?

Open tvvbbb opened this issue 7 years ago • 1 comments

https://github.com/googlesamples/android-architecture/blob/6419d4c523b67d020120fc400ed5a7372e5615f2/todoapp/app/src/main/java/com/example/android/architecture/blueprints/todoapp/data/source/TasksRepository.java#L299

In line 288, it judges the Map is null or not. Then new a map? If line 299 not like this, NullPointException will be trigged.

tvvbbb avatar Nov 25 '18 09:11 tvvbbb

Context: In the TasksRepository.java file of the Android Architecture sample (todoapp), there's a check on line 288:

if (mCachedTasks == null) { mCachedTasks = new LinkedHashMap<>(); } Question: Why is this null check and initialization needed before calling mCachedTasks.clear() on line 299, while deleteAllTasks() might not do the same?

Concern: Without the null check, calling methods like mCachedTasks.clear() would result in a NullPointerException.

Explanation You're absolutely right — this is a defensive programming practice.

deleteAllTasks() might ensure the cache is already initialized, or might not access mCachedTasks.clear() if it's null.

In contrast, the code near line 299 explicitly ensures the cache is not null before calling clear().

The logic is:

if (mCachedTasks == null) { mCachedTasks = new LinkedHashMap<>(); } mCachedTasks.clear(); // Safe now Without this, if mCachedTasks were still null, the call to .clear() would throw a NullPointerException.

Recommendation No action needed unless the deleteAllTasks() method has inconsistent or unsafe behavior. If you suspect that, it might be worth aligning both methods to use the same safe pattern for clarity and robustness.

VaradGupta23 avatar Jul 21 '25 09:07 VaradGupta23