google / google/ground-android

[Code health] Idea: Simplify local db interfaces and impls

Open
#1,459 0 comments 0 reactions 0 assignees View on GitHub
type: code health
Dominant language
Kotlin
Stars
293
Forks
149
Avg merge
1d 16h
Merged PRs (30d)
42

Description

The current structure of `ground/src/main/java/com/google/android/ground/persistence/local` is:

```
├── LocalDataConsistencyException.kt
├── LocalDataStore.kt
├── LocalDataStoreModule.kt
├── LocalDatabaseModule.kt
├── LocalValueStore.kt
├── room
│   ├── IntEnum.java
│   ├── LocalDataStoreException.kt
│   ├── LocalDatabase.kt
│   ├── RoomLocalDataStore.kt
│   ├── converter
│   │   ├── ConverterExt.kt
│   │   ├── JsonArrayTypeConverter.kt
│   │   ├── JsonObjectTypeConverter.kt
│   │   ├── ResponseDeltasConverter.kt
│   │   ├── ResponseJsonConverter.kt
│   │   ├── ResponseMapConverter.kt
│   │   └── StyleTypeConverter.kt
│   ├── dao
│   │   ├── BaseDao.kt
│   │   ├── BaseMapDao.kt
│   │   ├── JobDao.kt
│   │   ├── LocationOfInterestDao.kt
│   │   ├── LocationOfInterestMutationDao.kt
│   │   ├── MultipleChoiceDao.kt
│   │   ├── OfflineAreaDao.kt
│   │   ├── OptionDao.kt
│   │   ├── SubmissionDao.kt
│   │   ├── SubmissionMutationDao.kt
│   │   ├── SurveyDao.kt
│   │   ├── TaskDao.kt
│   │   ├── TileSetDao.kt
│   │   └── UserDao.kt
│   ├── entity
│   │   ├── AuditInfoEntity.kt
│   │   ├── BaseMapEntity.kt
│   │   ├── GeometryEntity.kt
│   │   ├── JobEntity.kt
│   │   ├── LocationOfInterestEntity.kt
│   │   ├── LocationOfInterestMutationEntity.kt
│   │   ├── MultipleChoiceEntity.kt
│   │   ├── OfflineAreaEntity.kt
│   │   ├── OptionEntity.kt
│   │   ├── SubmissionEntity.kt
│   │   ├── SubmissionMutationEntity.kt
│   │   ├── SurveyEntity.kt
│   │   ├── TaskEntity.kt
│   │   ├── TileSetEntity.kt
│   │   └── UserEntity.kt
│   ├── models
│   │   ├── Coordinates.java
│   │   ├── EntityState.java
│   │   ├── GeometryType.kt
│   │   ├── MultipleChoiceEntityType.java
│   │   ├── MutationEntitySyncStatus.java
│   │   ├── MutationEntityType.java
│   │   ├── OfflineAreaEntityState.java
│   │   ├── TaskEntityType.java
│   │   ├── TileSetEntityState.java
│   │   └── UserDetails.java
│   ├── relations
│   │   ├── JobEntityAndRelations.kt
│   │   ├── SurveyEntityAndRelations.kt
│   │   └── TaskEntityAndRelations.kt
│   └── stores
│   ├── RoomLocalLocationOfInterestMutationStore.kt
│   ├── RoomLocalOfflineAreaStore.kt
│   ├── RoomLocalSubmissionMutationStore.kt
│   ├── RoomLocalSurveyStore.kt
│   ├── RoomLocalTileSetStore.kt
│   └── RoomLocalUserStore.kt
└── stores
├── LocalLocationOfInterestMutationStore.kt
├── LocalMutationStore.kt
├── LocalOfflineAreaStore.kt
├── LocalStore.kt
├── LocalSubmissionMutationStore.kt
├── LocalSurveyStore.kt
├── LocalTileSetStore.kt
└── LocalUserStore.kt
```

A few questions that might help understand how to simplify this structure:
- [ ] If Stores are 1:1 with Daos, what value-add do they provide?
- [ ] At least one operation in `LocalDataStore` combines data from multiple model-based stores. If that's the case, should `LocalDataStore` be scoped to coordination layer rather than domain-model (i.e., implementation of local persistence vs. implementation of local persistence of a specific item)?
- [ ] Does the addition of multiple interfaces and related implementations for each domain object increase or decrease readability and complexity? Is it obvious when to look in `LocalDataStore`, `RoomLocalDataStore`, `LocalStore`, `LocalMutationStore`, `RoomLocalSurveyStore`, or `LocalSurveyStore`, for example?
- [ ] What are "models" and how do they differ from "entities"? If they're just subcomponents of entities, should they live alongside them in the same package?

@scolsen and I were discussing most of these today, @JSunde @shobhitagarwal1612 curious to hear your thoughts.

Note that I'm also having trouble adding coroutines gradually since use of Rx is prescribed by the shared interface.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.