# Care.com Android Technical Debt and Overview ## Intro In this document we are going to talk about initial review of the source code of the Care.com Android App, we are going to mentions some issues and suggestions to handle them. The objective of the review is finding a way forward that would allow Android project/team to move faster when introducing new features while at the same time keeping the highest level of quality for the project code and structure. --- ### Code Issues and suggestions - First of all, the code is not fully kotlin. We still have many java classes and code that need to be refactored to kotlin. Easier to call in kotlin code, maintain and use kotlin features. - The DI graph is not clear in the project, where we have much code for the DI, i think we need to refactor it and use hilt instead of dagger. - We have application life cycles and activities life cycles inside application and activity clases, where we can separate them in another classes, this will reduce class coupline and achive single responsibility princible. Making the code more readable and maintainable - We have many commented code in the project, in additioin to extra documentaiton and author/rights code. Actually we can get rid of all that commented code. We already have git in the project that will give us all infor about author of the code and commented code. - We have many classes, functions and code that aren't used any more in the code, we need to get rid of all that extra code which's what we are doing right now :rocket: - I noticed that we are still using Serializable interface in the project, i think we need to stop using it and move to Parcelable -android interface- which's faster and better for passing objects in android. - I think we have extra usage of inheritance in the projejct, like that one used for adapters like `JobTitleAdapter`. Inheritance is cool and everything, but it has its problems. - It encourages a gaggle of unwieldy subclasses. You can end up with many, many classes that may or may not meet your needs. - One never can be quite sure what the super-class is going to do. Perhaps it is in a module outside of your control, and the superclass is very heavy with many dependencies. Creating it causes side-effects that you don’t want or aren’t prepared for. - Class hierarchies are very hard to change once they’ve been deployed. Composition thus becomes preferable, where we can inject what we need for adapters. - No need to pass fragments and view models to list adapters, this will increase the code coupling and may cause memore leaks. Instead we can just observe the data from view model and submit these data to the list adapter to handle it, we can use callbacks to interact with the view. and for the context we can get it in the adapter once it's attached to the recycler view. This include the inheritance used in activity/fragments. - We shouldn't count on hard coded strings in navigation or checking conditions, for navigation we can use navigation library or implement a navigation manager that will be responsible on navigation handling. For others strings usage, we have to use enums, sealed classes and constants instead of hard coded strings. this's will increase the code readability, maintainability and testability. - We are using too many activities, actually we should use fragments and try to go to the single activity approach. - I see we are using live data in repository layer. I’d recommend using LiveData only for communication between ViewModels and the UI Layer where observers being called on the main thread makes more sense. As in LiveData documentation, the LiveData observers are always called on the main thread. Using them in repository layer may make many mani thread blocks in the app. I think It's better to use suspend functions and flows instead of using live data in repositories. Actually we shouldn’t bring Android stuff in our Domain layer, this's another reason to stop using live data in repository/domain layer. - Much code was implemented in views(activities/fragemnts) wehre the code logic and data handling should be in the repository layer or view models not views. We can use useCase pattern for complicated cases. - Custom views are something good for ui logic code wrapping, but some of the views are extra or over. where we can use extension functions and xml selectors to handle state changes. Easier to use and manage instead of creating custom views for each case. Using xml selectors with default android views will give us more flexibility to use these views in different places. - I think we need to enhance service and network calls in some areas like `SeekerEnrollmentWebService`, i think we can use retrofit and inject the required service using hilt/dagger - We have much code in the bind functions in recycler view adapters, we can create view holders and move bind code to bind function inside the view holder, to have single responsibility and high cohesion code. - We should abstract and hide the local storage implementations, we can't expose preferences in managers classes. Instead we can have a data interface to save/get data, preferences will be injected in the implementation class of that data interface class. - Factory pattern could be used to get hoopla views,so that we depend on featureFlagComponent to decide with view should be used. ### Code style and format The code doesn't follow android kotlin style code, in addition to extra lines and comments in the code. We need to follow one style in the whole project to ease reviewing, refactoring and navigation processes in the code. We can follow this [guide](https://developer.android.com/kotlin/style-guide?gclsrc=aw.ds&gclid=Cj0KCQiAzMGNBhCyARIsANpUkzOxpyus-qPBOZE3IXFE1yq3cYRquUjWLB2eh93-4WwPFfNfvQZ7aBAaAiAXEALw_wcB) from google and apply ktlint check tool in the project.