diff --git a/.gitignore b/.gitignore index c397484af0..24becfd9ad 100644 --- a/.gitignore +++ b/.gitignore @@ -48,6 +48,7 @@ captures/ .idea/dictionaries .idea/libraries .idea/deploymentTargetDropDown.xml +.idea/checkstyle-idea.xml #misc.xml is annoying and useless .idea/misc.xml # Android Studio 3 in .gitignore file. diff --git a/docs/Guidelines.md b/docs/Guidelines.md index 19f5a3d454..2241e61d79 100644 --- a/docs/Guidelines.md +++ b/docs/Guidelines.md @@ -5,7 +5,7 @@ The biggest challenge in software engineering is the fight against **complexity* 1. Your PR works and doesn't break anything. 2. Your PR is simple and doesn't add complexity. -Software engineering is also about **thinking**. Don't just follow blindly best practices and strive to do the "right" thing. Instead, ask yourself: +Software engineering is also about **thinking**. Don't just blindly follow best practices and strive to do the "right" thing. Instead, ask yourself: - Is this the simplest solution? - Am I over-engineering? What does this give me? diff --git a/docs/guidelines/Architecture.md b/docs/guidelines/Architecture.md index 33db94427f..f904df56c2 100644 --- a/docs/guidelines/Architecture.md +++ b/docs/guidelines/Architecture.md @@ -6,9 +6,9 @@ Ivy Wallet follows a less constrained version of [the official Android Developer ![data-mapping](../assets/data-mapping.svg) -> "Programming is a game of information. We receive/send data on which we perform arbitrary transformations and logic." — Iliyan Germanov +> "Programming is a game of information. We receive and send data, on which we perform arbitrary transformations and logic." — Iliyan Germanov -**Architecture:** _Data Layer → Domain Layer (optional) → UI layer_ +**Architecture:** _Data layer → Domain layer (optional) → UI layer_ ![architecture](../assets/architecture.svg) @@ -18,21 +18,21 @@ The Data Layer is responsible for dealing with the outside world and mapping it ### Data source (optional) -Wraps an IO operation (e.g. a Ktor http call) and ensures that it won't throw exceptions by making it a total function (i.e. wraps with `try-catch` and returns `Either` of some raw data model). +Wraps an IO operation (e.g., a Ktor http call) and ensures that it won't throw exceptions by making it a total function (i.e. wraps with `try-catch` and returns `Either` of some raw data model). > A data source isn't always needed if it'll do nothing useful. For example, there's no point wrapping Room DB DAOs. ### Domain Mapper classes (optional) -A classes responsible for transforming and validating raw models (e.g. DTOs, entities) to domain ones. These validations can fail so mappers usually return `Either`. +A class responsible for transforming and validating raw models (e.g., DTOs, entities) to domain ones. These validations can fail, so mappers usually return `Either`. ### Repository -Combines one or many data sources to implement [CRUD operations](https://en.wikipedia.org/wiki/Create,_read,_update_and_delete) and provide validated domain data. Repository functions must be **main-safe** (not blocking the main UI thread) or simply said they must move work on a background thread (e.g. `withContext(Disparchers.IO)`) +Combines one or many data sources to implement [CRUD operations](https://en.wikipedia.org/wiki/Create,_read,_update_and_delete) and provide validated domain data. Repository functions must be **main-safe** (not blocking the main UI thread), or simply said, they must move work on a background thread (e.g., `withContext(Disparchers.IO)`) ## Domain Layer (optional) -Optional architecture layer for more complex domain logic that combines one or many repositories with business logic and rules (e.g. calculating the balance in Ivy Wallet). +Optional architecture layer for more complex domain logic that combines one or many repositories with business logic and rules (e.g., calculating the balance in Ivy Wallet). ### UseCases @@ -44,16 +44,16 @@ The user of the app sees and interacts only with the UI layer. The UI layer cons ### ViewModel -The ViewModel combines the data from uses-cases and repositories and transforms it into view-state representation that's formatted and ready to display in your Compose UI. It also handles user interactions and translates them into data/domain layer calls. +The ViewModel combines the data from use cases and repositories and transforms it into view-state representation that's formatted and ready to display in your Compose UI. It also handles user interactions and translates them into data/domain layer calls. > Simply said, the viewmodel is translator between the UI (user) and the domain. It's like an adapter - adapts domain models to view-state and adapts user interactions into domain calls. ### ViewState Mapper classes (optional) -In more complex cases, it becomes impractical to put all domain -> view-state mapping in the ViewModel. Also, it's common multiple viewmodels to map the same domain model to the same view-state. In that case, it's good to extract the view-state mapping logic in a separate class that we call a `SomethingViewStateMapper`. +In more complex cases, it becomes impractical to put all domain -> view-state mapping in the ViewModel. Also, it's common for multiple viewmodels to map the same domain model to the same view-state. In that case, it's good to extract the view-state mapping logic in a separate class that we call a `SomethingViewStateMapper`. ### Composables Composables are the screens and UI components that the user sees and interacts with. They should be dumb as fck. Their responsibility and logic should be limited to: -- displaying the already formatted view-state provided by the VM -- send UI interactions to the VM in the form of events +- Displaying the already formatted view-state provided by the VM. +- Sending UI interactions to the VM in the form of events. diff --git a/docs/guidelines/Data-Modeling.md b/docs/guidelines/Data-Modeling.md index f0a9e57db5..12c25665ab 100644 --- a/docs/guidelines/Data-Modeling.md +++ b/docs/guidelines/Data-Modeling.md @@ -23,9 +23,8 @@ The problem with this approach is that our code will have to deal with many impo - What to show if `loading = false`, `content = null`, `error = null`? - What to do if we have both `loading = true` and `error != null`? -There are so many ways things to go wrong - for example, a common one is forgetting to reset `loading` back to `false`. -A better way to model this would be to use [Algebraic Data types (ADTs)](https://wiki.haskell.org/Algebraic_data_type) -or simply said in Kotlin: `data classes`, `sealed interfaces`, and combinations of both. +There are so many ways things can go wrong - for example, a common one is forgetting to reset `loading` back to `false`. +A better way to model this would be to use [Algebraic Data types (ADTs)](https://wiki.haskell.org/Algebraic_data_type) or, simply said in Kotlin: `data classes`, `sealed interfaces`, and combinations of both. ```kotlin sealed interface ScreenUiState { @@ -35,7 +34,7 @@ sealed interface ScreenUiState { } ``` -With the ADTs representation, we eliminate all impossible cases. We also do eliminate that on compile-time, meaning that whatever shit we do - the compiler will never allow the code to run. +With the ADTs representation, we eliminate all impossible cases. We also eliminate that at compile-time, meaning that whatever shit we do - the compiler will never allow the code to run. **Takeaway:** Model your data using `data classes`, and `sealed interfaces` (and combinations of them) in a way that: @@ -60,8 +59,8 @@ data class Order( ) ``` -I'm making this up but the goal is to demonstrate common mistakes and how to fix them. -Do you spot them? +I'm making this up, but the goal is to demonstrate common mistakes and how to fix them. +Do you spot them? Let's think and analyze: @@ -69,7 +68,7 @@ Let's think and analyze: 2. Imagine a function `placeOrder(orderId: UUID, userId: UUID, itemId: UUID, ...)`. How likely is someone to pass a wrong `UUID` and mess UUIDs up? 3. The `trackingId` seems to be required and important but what if someone passes `trackingId = ""` or `trackingId = "XYZ "`? -I can go on but you see the point. So let's how we can fix it. +I can go on, but you see the point. So let's discuss how we can fix it. ```kotlin data class Order( @@ -109,7 +108,7 @@ PositiveInt.from(-5) // Either.Left("-5 is not > 0") ``` -The revised data model takes more code but it gives you one important property: +The revised data model takes more code, but it gives you one important property: > If any of your functions accepts an instance of `order: Order`, you immediately know that it's a valid order and no validation logic is required. @@ -118,9 +117,9 @@ This is **validation by construction** and it eliminates undesirable cases asap - Order `count` of zero, negative, or infinity by explicitly requiring a `PositiveInt` (unfortunately, that happens at runtime because the compiler can't know if a given integer is positive or not by just looking at the code). - The `UUID`s now can't be messed up because the compiler will give you an error, if for example you try to pass `UserId` to a function accepting `OrderId`. - The `time` is now always in UTC by using `Instant`. -- The `trackignId` is trimmed and can't be blank. +- The `trackingId` is trimmed and can't be blank. To learn more about Exact types you can check [the Arrow Exact GitHub repo](https://github.com/arrow-kt/arrow-exact). The benefit of explicit data models is correctness and reduced complexity of your core logic. > Not all types should be exact. For example, we make an exception for DTOs and entities where working with primitives is easier. -> However, we still use ADTs and everything in the domain layer where the business logic is must be exact and explicit. +> However, we still use ADTs and everything in the domain layer where the business logic must be exact and explicit. diff --git a/docs/guidelines/Error-Handling.md b/docs/guidelines/Error-Handling.md index 30b2d385d5..7986901eb0 100644 --- a/docs/guidelines/Error-Handling.md +++ b/docs/guidelines/Error-Handling.md @@ -1,11 +1,10 @@ # Error Handling It's common for operations to fail and we should expect that. -In Ivy Wallet we **do not throw exceptions** but rather make functions that -can fail return [Either](https://arrow-kt.io/learn/typed-errors/working-with-typed-errors/). +In Ivy Wallet we **do not throw exceptions** but rather make functions that can fail to return [Either](https://arrow-kt.io/learn/typed-errors/working-with-typed-errors/). Either is a generic data type that models two possible cases: -- `Either.Left` for the unhappy path (e.g. request failing, invalid input, no network connection) +- `Either.Left` for the unhappy path (e.g., request failing, invalid input, no network connection) - `Either.Right` for the happy path Simplified, `Either` is just: @@ -24,7 +23,7 @@ fun Either.fold( Either.Right -> mapRight(data) } -// a bunch more extension functions and utils +// a bunch of more extension functions and utils ``` So in Ivy, operations that can fail (logically or for some other reason) we'll model using **Either**. @@ -45,7 +44,7 @@ interface BtcDataSource { } interface MyBank { - suspend fun currentblBalanceUSD(): Either + suspend fun currentBalanceUSD(): Either } class CryptoInvestor @Inject constructor( @@ -54,7 +53,7 @@ class CryptoInvestor @Inject constructor( ) { suspend fun buyIfCheap(): Either = either { val btcPrice = btcDataSource.fetchCurrentPriceUSD().bind() - // .bind() - if it fails returns Either.Left and short-circuits the function + // .bind() - if it fails, returns Either.Left and short-circuits the function if(btcPrice.value > 50_000) { // short-circuits and returns Either.Left with the msg below raise("BTC is expensive! Won't buy.") @@ -76,7 +75,7 @@ class CryptoInvestor @Inject constructor( Let's analyze, simplified: - `either {}` puts us into a "special" scope where the last line returns `Either.Right` and also gives us access to some functions: - - `Operation.bind()`: if the operation fails terminates the `either {}` with operation's `Left` value, otherwise `.bind()` returns the operation's `Right` value + - `Operation.bind()`: if the operation fails, it terminates the `either {}` with operation's `Left` value; otherwise, `.bind()` returns the operation's `Right` value - `raise(E)`: like **throw** but for `either {}` - terminates the function with `Left(E)` - `Either.mapLeft {}`: transforms the `Left` (error type) of the `Either`. In the example, we do it so we can match the left type of the `either {}` @@ -99,7 +98,7 @@ I strongly recommend allocating some time to also go through [Arrow's Working wi - Either is a [monad](https://en.wikipedia.org/wiki/Monad_(functional_programming)). - `Either` is equivalent to Kotlin's std `Result` type. -- Many projects create a custom `Result` while they can just use `Either` with all of its built-in features. +- Many projects create a custom `Result` while they can just use `Either` with all its built-in features. -> In some rare cases it's okay to `throw` a runtime exception. Those are the cases in which you're okay and want the app to crash -> (e.g. not enough disk space to write in Room DB / local storage). +> In some rare cases, it's okay to `throw` a runtime exception. These are the cases in which you're okay and want the app to crash +> (e.g., not enough disk space to write in Room DB / local storage). diff --git a/docs/guidelines/Screen-Architecture.md b/docs/guidelines/Screen-Architecture.md index 13b3b2885e..a614fffb9a 100644 --- a/docs/guidelines/Screen-Architecture.md +++ b/docs/guidelines/Screen-Architecture.md @@ -1,7 +1,6 @@ # Screen Architecture -Ivy Wallet uses an [Unidirectional Data Flow (UDF)](https://developer.android.com/topic/architecture#unidirectional-data-flow), -MVI architecture pattern with the Compose runtime for reactive state management in the view-model. +Ivy Wallet uses a [Unidirectional Data Flow (UDF)](https://developer.android.com/topic/architecture#unidirectional-data-flow) and MVI architecture pattern with the Compose runtime for reactive state management in the view-model. It key characteristics are: ![screen-architecture](../assets/screen-vm.svg) @@ -15,22 +14,22 @@ Repeat ♻️ ## ViewModel -A class that adapts the domain model to view-state model that the Compose UI can directly display. It combines data from one or many repositories/use-cases and transforms it into a view-state representation consisting of primitives and `@Immutable` structures that composables can draw efficiently. +A class that adapts the domain model to a view-state model that the Compose UI can directly display. It combines data from one or many repositories/use-cases and transforms it into a view-state representation consisting of primitives and `@Immutable` structures that composables can draw efficiently. -Let's address the elephant in the room, why Compose in the ViewModel? The short answer, because it's way more convenient and equally efficient compared to using Flow/LiveData. +Let's address the elephant in the room - why use Compose in the ViewModel? The short answer: because it's way more convenient and equally efficient compared to using Flow/LiveData. ### FAQ **Q: Isn't it an anti-pattern to have Compose and Android/UI logic in the view-model?** -A: Firstly, Compose is more modular than it looks on the surface. `compose.runtime` is very different from the `compose.ui`. In our architecture we use only the Compose runtime as a reactive state management library. The compose runtime state is equivalent to Kotlin Flow but with simpler, more elegant and powerful API for the purposes of a view-model. +A: Firstly, Compose is more modular than it looks on the surface. `compose.runtime` is very different from the `compose.ui`. In our architecture we use only the Compose runtime as a reactive state management library. The compose runtime state is equivalent to Kotlin Flow but with a simpler, more elegant and powerful API for the purposes of a view-model. **Q: Don't we couple our view-models with Compose by doing this?** A: In theory, we couple our ViewModel only with the Compose runtime and its compiler. However, that doesn't matter because: 1. Let's admit it, you'll likely won't change Compose as your UI toolkit anytime soon. -2. If you change Compose, rewriting the UI compsables and components will cost you much more than migrating your view-models because viewmodels if done correctly are very simple adapters of your data/domain layer. +2. If you do change Compose, rewriting the UI composables and components will cost you much more than migrating your view-models, because viewmodels, if done correctly, are very simple adapters of your data/domain layer. **Q: Can we use Kotlin Flow APIs in a compose viewmodel?** @@ -46,7 +45,7 @@ fun getBtcPrice(): String? { **Q: What's the benefit of having Compose in the VM?** -A: The main benefit is convenience. With the Compose runtime you don't have to do complex Flows like `combine` (limited to 5 flows only), `flapMapLatest` vs `flatMapCombine` and write all the boilerplate code required. Another benefit is that you also have access to entire Compose runtime API like `remember` (easy memorization), `LaunchedEffect` (execute side-effects under certain conditions) and ofc simple, concise and very readable syntax. +A: The main benefit is convenience. With the Compose runtime you don't have to do complex Flows like `combine` (limited to 5 flows only), `flapMapLatest` vs `flatMapCombine` and write all the boilerplate code required. Another benefit is that you also have access to the entire Compose runtime API like `remember` (easy memorization), `LaunchedEffect` (execute side-effects under certain conditions), and, ofc, simple, concise, and very readable syntax. All of the above is better seen in code and practice - make sure to check our references to learn more. @@ -58,11 +57,11 @@ The view-state is a data model that contains all information that the screen/com ## View-event -Our users need to be able to interact with the app and its Compose UI. Those interactions include typing input, clicking buttons, gestures and more. The Compose UI captures those interactions and maps them into view-events that the view-model can easily handle and process. +Our users need to be able to interact with the app and its Compose UI. These interactions include typing input, clicking buttons, gestures, and more. The Compose UI captures these interactions and maps them into view-events that the view-model can easily handle and process. ## Composable UI -The Compose UI is responsible for rendering the view-state according to its design and allowing the user to interact with the UI. The Compsable UI listens for user interactions and maps to events that it sends to the VM +The Compose UI is responsible for rendering the view-state according to its design and allowing the user to interact with the UI. The Composable UI listens for user interactions and maps them to events that it sends to the VM. ## References diff --git a/docs/guidelines/Unit-Testing.md b/docs/guidelines/Unit-Testing.md index 1b136a4421..9e0a2134ec 100644 --- a/docs/guidelines/Unit-Testing.md +++ b/docs/guidelines/Unit-Testing.md @@ -2,17 +2,17 @@ Unit tests are the proof that your code works. While often seen as boring and a waste of time, automated tests are the things that guarantee correctness under the test cases and -assumptions that you've setup. +assumptions that you've set up. -A good unit test is short and simple. If you're test case doesn't fit -on half the screen then it's likely bad. Also, if you can't understand what's happening +A good unit test is short and simple. If your test case doesn't fit +on half the screen then, it's likely bad. Also, if you can't understand what's happening in a test at a glance, then it's bad again. ## Unit test structure Most good unit tests share a similar structure/pattern. They start with a simple name that reads like a sentence and tells you what's being tested. Then inside -the test functions body, they're split into three parts. +the test function's body, they're split into three parts. ```kotlin class CurrencyConverterTest { @@ -63,39 +63,39 @@ class CurrencyConverterTest { ### Given (optional) Here we do the required setup for our test case. This usually involves: -- mocking stuff (e.g. `every { x.something() } returns Y`). +- mocking stuff (e.g., `every { x.something() } returns Y`). - creating instances of data classes required for the test. - other preparatory work that we need to set the **sut (system under test)** -to the correct state to test our **code path under test** (e.g. happy path, unhappy path, edge-case, etc). +to the correct state to test our **code path under test** (e.g. happy path, unhappy path, edge case, etc.). ### When -In the `// when` section we execute the code path under test or i.e. just call the function we want to test with the arguments needed. +In the `// when` section, we execute the code path under test, i.e., just call the function we want to test with the arguments needed. It's also good practice to save the result of the function in a `val res = ...` that you'll verify in the `// then` section. ### Then -Lastly in the `// then` section, we verify the result of the test and make assertions. We can assert that: +Lastly, in the `// then` section, we verify the result of the test and make assertions. We can assert that: -- the returned result is what we expect using `res shouldBe X` -- the side effects that we expected happened (e.g. `coVerify(exactly = 1) { repo.save(item) }`) -- side effects that shouldn't happen didn't happen (`coVerify(exactly = 0) { repo.delete(any()) }`) -- other affected systems state is what we expect (e.g. `fakeUserDao.getCurrentUser shouldBe user1`) +- The returned result is what we expect using `res shouldBe X`. +- The side effects that we expected happened (e.g., `coVerify(exactly = 1) { repo.save(item) }`). +- Side effects that shouldn't happen didn't happen (e.g., `coVerify(exactly = 0) { repo.delete(any()) }`). +- Other affected systems' state is what we expect (e.g., `fakeUserDao.getCurrentUser shouldBe user1`). -Here you should be smart and be careful to assert only the things that you care about. -Too many assertions and the test becomes a pain in the ass to maintain. -Too few assertions and the test becomes useless. +Here, you should be smart and be careful to assert only the things that you care about. +Too many assertions, and the test becomes a pain in the ass to maintain. +Too few assertions, and the test becomes useless. -> As a rule of thumb, test the function as if it's a black box and you don't know its implementation. +> As a rule of thumb, test the function as if it's a black box, and you don't know its implementation. > Assert only the things that you expect from its signature and responsibility without looking at the implementation. ## When to mock and when not? -That solely depends on whether you want to execute the code paths in a dependency. -It's common in practice for some code paths to be impossible to execute in JVM unit tests - for example using real Room DB DAOs and SQLite database. +That solely depends on whether you want to execute the code paths in a dependency. +It's common in practice for some code paths to be impossible to execute in JVM unit tests - for example, using real Room DB DAOs and SQLite database. In those cases, we have two options: -- create fake implementations (e.g. `FakeDao`, `FakeDataStore` that stores stuff in-memory only) -- mock them using [MockK](https://mockk.io/) +- Create fake implementations (e.g., `FakeDao`, `FakeDataStore` that stores stuff in-memory only) +- Mock them using [MockK](https://mockk.io/) There's not a single rule but I'll give you some questions to help you decide: 1. Do I want to test and execute the code path (implementation) of this dependency? @@ -112,4 +112,4 @@ It's all about making your tests short, simple, and valuable. - To create similar `data classes` and models to the already extracted ones use `.copy()` or create those extracted objects using a `fun` that accepts default arguments. - If the same helper `vals` and util functions are needed in different test classes, extract them in a new `object SomethingFixtures` file so they can be reused. - If your test is still not fitting half the screen, maybe you're testing too much in a single test case. Split the test into multiple smaller tests that verify only a single portion of the original large one. -- Sometimes when the `data class` you want to create is too hard to create you can even mock it like `val transaction = mockk(relaxed = true) { every { settled } returns true }` to simplify your test. +- Sometimes when the `data class` you want to create is too hard to create, you can even mock it like `val transaction = mockk(relaxed = true) { every { settled } returns true }` to simplify your test.