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

[todo-mvp] Add TasksCacheDataSource

Open jeffmcnd opened this issue 7 years ago • 1 comments

I've noticed that the cache and the caching logic is in the TasksRepository class which, I believe, violates the single responsibility principle. Would it be worth it to create an abstraction for the cache by adding another class? It wouldn't follow the TasksDataSource interface as its functionality is a little different but it would remove a responsibility from TasksRepository and get rid of a lot of if statements that are used for null checks.

I'm going to implement a version of this and see if it works.

jeffmcnd avatar Mar 06 '18 21:03 jeffmcnd

Right — in the todo-mvp architecture, the TasksRepository currently plays two roles:

Repository – coordinating between local (DB) and remote data sources.

In-memory cache – storing the already-fetched Task objects in a LinkedHashMap with null-checking logic.

That’s convenient for a small sample, but as @jeffmcnd pointed out in that GitHub issue (#522), it does bend the Single Responsibility Principle.

Why moving the cache out makes sense Simplifies TasksRepository – it should only decide where to get data from, not how to hold it in memory.

Easier testing – you can test caching separately without having to spin up the full repository logic.

Easier replacement – later you could swap an in-memory cache for something like a Room in-memory database or a disk cache without touching the repository.

How it could look

  1. Create a new TasksCacheDataSource

public class TasksCacheDataSource {

private final Map<String, Task> mCachedTasks = new LinkedHashMap<>();
private boolean mCacheIsDirty = false;

public void saveTasks(List<Task> tasks) {
    mCachedTasks.clear();
    for (Task task : tasks) {
        mCachedTasks.put(task.getId(), task);
    }
    mCacheIsDirty = false;
}

public void addTask(Task task) {
    mCachedTasks.put(task.getId(), task);
}

public void markCacheDirty() {
    mCacheIsDirty = true;
}

public boolean isCacheDirty() {
    return mCacheIsDirty;
}

public List<Task> getCachedTasks() {
    return new ArrayList<>(mCachedTasks.values());
}

public Task getTask(String taskId) {
    return mCachedTasks.get(taskId);
}

public void clearCache() {
    mCachedTasks.clear();
}

} 2. Update TasksRepository to use it

public class TasksRepository implements TasksDataSource {

private static TasksRepository INSTANCE = null;

private final TasksDataSource mTasksRemoteDataSource;
private final TasksDataSource mTasksLocalDataSource;
private final TasksCacheDataSource mTasksCacheDataSource;

private TasksRepository(@NonNull TasksDataSource tasksRemoteDataSource,
                        @NonNull TasksDataSource tasksLocalDataSource,
                        @NonNull TasksCacheDataSource tasksCacheDataSource) {
    mTasksRemoteDataSource = checkNotNull(tasksRemoteDataSource);
    mTasksLocalDataSource = checkNotNull(tasksLocalDataSource);
    mTasksCacheDataSource = checkNotNull(tasksCacheDataSource);
}

public static TasksRepository getInstance(TasksDataSource tasksRemoteDataSource,
                                          TasksDataSource tasksLocalDataSource,
                                          TasksCacheDataSource tasksCacheDataSource) {
    if (INSTANCE == null) {
        INSTANCE = new TasksRepository(tasksRemoteDataSource, tasksLocalDataSource, tasksCacheDataSource);
    }
    return INSTANCE;
}

@Override
public void getTasks(@NonNull LoadTasksCallback callback) {
    if (!mTasksCacheDataSource.isCacheDirty()) {
        List<Task> cachedTasks = mTasksCacheDataSource.getCachedTasks();
        if (!cachedTasks.isEmpty()) {
            callback.onTasksLoaded(cachedTasks);
            return;
        }
    }
    // Fetch from local/remote as before, then update cache via mTasksCacheDataSource.saveTasks()
}

// ...other repository methods use mTasksCacheDataSource for caching...

} Benefits TasksRepository now only coordinates data retrieval between sources and updates the cache — but doesn’t hold the caching logic itself.

TasksCacheDataSource becomes reusable if another repository in the app needs in-memory caching.

Null-check-heavy code is moved into one place.

VaradGupta23 avatar Aug 08 '25 12:08 VaradGupta23