Photo by Joseph James: Basking in the Sun: A Cyclist’s Serene Wait

Eliminating Common Pitfalls in Clean Architecture Implementation for Android

Introduction

Implementing a clean architecture in Android development brings numerous benefits, such as code maintainability, testability, and scalability. However, developers often encounter pitfalls that can hinder the effectiveness of their clean architecture implementation. In this blog post, we will explore common pitfalls, provide insights on overcoming them, and discuss best practices for Android clean architecture.

Pitfall 1

Violating the Single Responsibility Principle (SRP)
One common pitfall is violating the SRP by having modules that take on multiple responsibilities. This leads to tightly coupled and hard-to-maintain code. Embracing the SRP promotes modularity and code maintainability, resulting in more readable unit tests and reduced complexity within a class.

Let me try to explain with an example, Let’s say we have the class following class:

internal class LogAnalyser @Inject constructor(){
    fun analyze(rawLogData:RawLogData){
        val processedLogs = processLogs(rawLogData)
        return detectErrors(processedLogs)
    }

    fun detectErrors(processedLog: ProcessedLog) : ErrorLogs {
        val errorLogs = TODO("do some processing on processedLog and extract the errors")
        return errorLogs
    }
    private fun processLogs() : ProcessedLog {
        // Do deterministic operations on the long list of logs here
        val processedLog: ProcessedLog = TODO("may be an operation on a different thread")
        return processedLog
    }
}

The LogAnalyser class needs some improvements from a code reviewer’s perspective. It performs two tasks: processing logs into a readable form and extracting errors from the processed logs. To adhere to the Single Responsibility Principle, it would be better to split these tasks into separate classes. Additionally, the code would benefit from clearer naming conventions. Furthermore, the unit test classes appear to be lengthy, which can make them harder to review. Breaking them down into smaller, focused tests would be beneficial.

Solution: Identify and separate responsibilities into distinct modules or classes, ensuring each has a single responsibility. This separation improves code organization, making it easier to understand, test, and maintain. By having smaller, focused classes with clear responsibilities, unit tests become more readable and the complexity of the logic within each class is significantly reduced. This enables developers to isolate and test individual functionalities more effectively, enhancing the overall quality of the codebase.

internal class GetProcessedLogUseCase @Inject constructor() {
    operator fun invoke(rawLogData: RawLogData): ProcessedLog {
        // Do deterministic operations on the long list of logs here
        val processedLog: ProcessedLog = TODO("may be an operation on a different thread")
        return processedLog
    }
}

internal class GetErrorFromProcessedLogsUseCase @Inject constructor() {
    operator fun invoke(processedLog: ProcessedLog): ErrorLogs {
        val errorLogs = TODO("do some processing on processedLog and extract the errors")
        return errorLogs
    }
}

internal class GetErrorLogsUseCase @Inject constructor(
    private val getProcessedLog: GetProcessedLogUseCase,
    private val getErrorFromProcessedLogs: GetErrorFromProcessedLogsUseCase
) {
    fun analyze(rawLogData: RawLogData) : ErrorLogs {
        val processedLogs = getProcessedLog(rawLogData)
        return getErrorFromProcessedLogs(processedLogs)
    }
}

What’s the difference here? We now have well-segregated classes, each with a single, specific responsibility. This change enhances the code by ensuring that each class provides a complete context for its existence. What does that mean? It means we have transparent unit tests that are highly readable, making the lives of code reviewers easier. Additionally, renaming the parent context further improves readability, adding a +1 to the overall code quality.

Pitfall 2

Dependency Rule Violation
Clean architecture emphasizes the dependency rule, where dependencies should flow inward, with higher-level modules depending on lower-level ones. A common pitfall is unintentionally violating this rule, resulting in a tangled web of dependencies.

internal class GetErrorLogsUseCase @Inject constructor(
    private val getProcessedLog: GetProcessedLogUseCase,
    private val getErrorFromProcessedLogs: GetErrorFromProcessedLogsUseCase
) {
    
    private val _events = MutableSharedFlow<ErrorLogs>()
    val events = _events.asSharedFlow()    

    fun analyze(rawLogData: RawLogData) {
        val processedLogs = getProcessedLog(rawLogData)
        val errorLog = getErrorFromProcessedLogs(processedLogs)
        _events.tryEmit(ErrorLogState.Error(event))
    }
}

This is the extension of the previous example, Here I’m changing my approach slightly. Instead of returning the ErrorLog, we expose a SharedFlow so that the multiple subscribers can listen to the error events.

Let’s see what the code review process can bring here.

We have the variable events exposed from the UseCase. This is going to make the UseCase act like a source of truth for the error logs. This leads to tight coupling between the use case and the concrete data access components. This results in reduced testability, limited modularity, and violations of the Single Responsibility Principle

Solution: Review the dependency graph of your architecture and ensure dependencies adhere to the intended flow. Use techniques such as dependency inversion, dependency injection, or service locators to properly manage dependencies and maintain the integrity of your architecture.

internal class ErrorLogsRepository @Inject constructor(){
    private val _events = MutableSharedFlow<ErrorLogs>()
    val events = _events.asSharedFlow()
    
    fun updateEvents(errorLogs: ErrorLogs){
        _events.tryEmit(ErrorLogState.Error(errorLogs))
    }
}
internal class GetErrorLogsUseCase @Inject constructor(
    private val getProcessedLog: GetProcessedLogUseCase,
    private val getErrorFromProcessedLogs: GetErrorFromProcessedLogsUseCase,
    private val errorLogsRepository:ErrorLogsRepository
) {
    fun analyze(rawLogData: RawLogData) {
        val processedLogs = getProcessedLog(rawLogData)
        val errorLog = getErrorFromProcessedLogs(processedLogs)
        errorLogsRepository.updateEvents(errorLog)
    }
}

Here are the key improvements:

• Single Responsibility Principle ensured.
• Dependency inversion which provides better abstraction.
• Better code readability.
• Improved testability.
• Easier future changes for data source implementation.

These enhancements bring benefits in terms of code organization, flexibility, and maintainability. The code becomes more modular, facilitating understanding and maintenance. Dependency inversion reduces coupling to specific data access components, allowing for easier changes in the future. Additionally, testing is simplified, enabling independent testing of the use case.

Pitfall 3

Business Logic Leakage in Presentation Layer
Leaking business logic into the presentation layer undermines the separation of concerns and reduces modularity. The core business logic should remain isolated from UI-related operations.

internal class LogsDisplayViewModel @Inject constructor(
    private val getErrorLogs : GetErrorLogsUseCase
): ViewModel() {
    
    private val _errorEvents: MutableStateFlow<CleanErrors> = MutableStateFlow(CleanErrors.Idle)
    val errorEvents: StateFlow<CleanErrors> = _errorEvents
    
    init {
        observeLogs()
    }

    private fun observeLogs() {
        viewModelScope.launch {
            getErrorLogs().collect { errorLogs->
                val result = removeOutliersFromError(errorLogs)
                _errorEvents.tryEmit(result)
            }
        }
    }

    private fun removeOutliersFromError(errorLogs: ErrorLogs): CleanErrors {
        val cleanError = TODO("remove outliers")
        return cleanError
    }
}

In a reviewer’s perspecive, the Business Logic Leakage in Presentation Layer happens in the function removeOutliersFromError. This method performs complex calculations or transformations on the user data retrieved from the repository. These calculations may involve data manipulation, formatting, or extracting relevant information specific to the ErrorLogs.

Solution: Define clear boundaries between the business logic and presentation layers. Utilize intermediary layers, such as use cases or domain models, to encapsulate the business logic. This promotes maintainability and ensures the presentation layer remains focused on user interactions and UI rendering.

internal class GetOutlierRemovedLogsUseCase @Inject constructor(
    private val getErrorLogs: GetErrorLogsUseCase
) {
    operator fun invoke() : Flow<CleanErrors> = getErrorLogs().map { errorLog ->
        removeOutliersFromError(errorLog)
    }

    private fun removeOutliersFromError(errorLogs: ErrorLogs): CleanErrors {
        val cleanError = TODO("remove outliers")
        return cleanError
    }
}

internal class LogsDisplayViewModel @Inject constructor(
    private val getOutlierRemovedLogs: GetOutlierRemovedLogsUseCase
) : ViewModel() {

    private val _errorEvents: MutableStateFlow<CleanErrors> = MutableStateFlow(CleanErrors.Idle)
    val errorEvents: StateFlow<CleanErrors> = _errorEvents

    init {
        observeLogs()
    }

    private fun observeLogs() {
        viewModelScope.launch {
            getOutlierRemovedLogs().collect { errorLogs ->
                _errorEvents.tryEmit(errorLogs)
            }
        }
    }
}

What did we do here? We extracted the process of removing outliers from the error logs to the domain layer. This results in a more readable and self-explanatory class, which enhances code simplicity. Additionally, this extraction facilitates the addition of unit tests, further improving the quality and maintainability of the code.

Pitfall 4

Over-Reliance on Framework-Specific Libraries
Over-reliance on framework-specific libraries introduces tight coupling and hinders portability and flexibility. It limits the ability to adapt to evolving requirements or technology changes.

internal class FacialDataProcessorUseCase @Inject constructor(
    private val detectFaceInImage: DetectFaceInImage
) : ImageAnalysis.Analyzer {

    override fun analyze(image: ImageProxy) {
      // Do some processing and append values to the repository.
    }
}

Using a library like MlKit for face movement detection poses a concern in the domain layer. If a new version of the library introduces changes, both the presentation and domain layers may need simultaneous updates. To mitigate this, introduce an abstraction layer to decouple the domain layer from the library implementation. This ensures easier maintenance and adaptability to future changes.

Solution: Abstract framework-specific functionality behind interfaces or abstractions to decouple the application from specific implementation details. This allows for easy substitution of frameworks or libraries, making the application more adaptable and future-proof.

// We move this to the presentation layer.
internal class FacialDataProcessor @AssistedInject constructor(
    private val detectFaceInImage: DetectFaceInImage,
    @Assisted private val callBack: (FacialData) -> Unit
) : ImageAnalysis.Analyzer {

    @AssistedFactory
    interface Factory {
        fun create(callBack: (FacialData) -> Unit): FacialDataProcessor
    }

    override fun analyze(image: ImageProxy) {
        // Do some processing and append values to the repository.
    }
}

internal class FacialRecognitionActivity : AppCompatActivity() {
    @Inject
    private lateinit var facialDataProcessorFactory: FacialDataProcessor.Factory

    private val facialDataViewModel: FacialDataViewModel = TODO("createViewModel")

    override fun onCreate(savedInstanceState: Bundle?) {
        super.onCreate(savedInstanceState)

        setContent {
            RenderFacialRecognitionScreen(
                onFacialDataCollected = { facialData ->
                    facialDataViewModel.processFacialRecognitionData(facialData)
                },
                facialDataProcessor = facialDataProcessorFactory
            )
        }
    }
}

internal class FacialDataToFacialFeaturesMapper @Inject constructor() {
    operator fun invoke(facialData: FacialData): FacialFeatures {
        val facialFeatures = TODO("Do some mapping")
        return facialFeatures
    }
}

internal class FacialDataViewModel @Inject constructor(
    private val saveFacialRecognitionData: SaveFacialRecognitionDataUseCase,
    private val facialDataToFacialFeaturesMapper: FacialDataToFacialFeaturesMapper
) : ViewModel() {
    fun processFacialRecognitionData(facialData: FacialData) = saveFacialRecognitionData(
        facialDataToFacialFeaturesMapper(facialData)
    )
}

What did we do here? We made the following changes:

• Added the FacialDataProcessor to the presentation layer, as it depends on a specific library implementation.
• Achieved decoupling of the domain layer, bringing happiness 🌈. Now, we have a domain-specific implementation of the logic that remains unaffected by library updates.
• Facilitated easy unit testing ✨ of the domain layer with the isolated logic implementation.

Conclusion

Implementing clean architecture in Android development provides a solid foundation for building maintainable, testable, and scalable applications. By being aware of common pitfalls, such as violating the SRP, managing dependencies, preventing business logic leakage, and avoiding over-reliance on framework-specific libraries, developers can create robust and flexible architectures. Adhering to principles like the SRP and the dependency rule ensures modular and maintainable code. Let’s embrace clean architecture principles to build Android applications that are easier to maintain, evolve, and extend over time.