Improve Readability and Maintainability of Data Purge Function (purge())

[itstanany]

Describe the Issue

This issue proposes a refactoring of the purge function within the engine module for improved code readability and maintainability. The current implementation checks for resource presence, local changes, and force purge flag within nested conditional statements, potentially leading to less readable and maintainable code.

Suggested Changes:

The proposed refactor aims to achieve the same functionality with improved clarity by:

• Combined check for local changes and force purge: Combine the checks for local changes (localChanges.isNotEmpty()) and the forcePurge flag within a single condition.
• Concise comments: Add concise comments to explain the logic behind each step.
• Reduced redundancy:

Benefits:

• Improved code readability by separating concerns and using clear logic flow.
• Enhanced maintainability by simplifying conditional checks and reducing nesting.
• Potentially clearer code for future developers working on this functionality.

Attached Code Snippets:

Include the current and proposed code snippets within the issue description for easy reference:

Current Code:

override suspend fun purge(type: ResourceType, ids: Set<String>, forcePurge: Boolean) {
    db.withTransaction {
      ids.forEach { id ->
        // To check resource is present in DB else throw ResourceNotFoundException()
        selectEntity(type, id)
        val localChangeEntityList = localChangeDao.getLocalChanges(type, id)
        // If local change is not available simply delete resource
        if (localChangeEntityList.isEmpty()) {
          resourceDao.deleteResource(resourceId = id, resourceType = type)
        } else {
          // local change is available with FORCE_PURGE the delete resource and discard changes from
          // localChangeEntity table
          if (forcePurge) {
            resourceDao.deleteResource(resourceId = id, resourceType = type)
            localChangeDao.discardLocalChanges(
              token = LocalChangeToken(localChangeEntityList.map { it.id }),
            )
          } else {
            // local change is available but FORCE_PURGE = false then throw exception
            throw IllegalStateException(
              "Resource with type $type and id $id has local changes, either sync with server or FORCE_PURGE required",
            )
          }
        }
      }
    }
}

Proposed Code:

  override suspend fun purge(type: ResourceType, ids: Set<String>, forcePurge: Boolean) {
    db.withTransaction {
      ids.forEach { id ->
        // 1. Verify resource presence:
        selectEntity(type, id)

        // 2. Check for local changes and handle accordingly:
        val localChanges = localChangeDao.getLocalChanges(type, id)
        if (localChanges.isNotEmpty() && !forcePurge) {
          throw IllegalStateException(
            "Resource with type $type and id $id has local changes, either sync with server or FORCE_PURGE required"
          )
        }

        // 3. Delete resource and discard local changes (if applicable):
        resourceDao.deleteResource(id, type)
        if (localChanges.isNotEmpty()) {
          localChangeDao.discardLocalChanges(id, type)
        }
      }
    }
  }

This approach provides a clear and concise issue description for the Android FHIR repository, promoting collaboration and potential code improvement.

Would you like to work on the issue? Yes 🚀

[MJ1998]

Thanks @itstanany. This looks good to me!