Skip to content
This repository was archived by the owner on Nov 5, 2024. It is now read-only.

Commit 84938b1

Browse files
Guidelines (#3057)
* WIP: Reorganize the docs * WIP: Start working on the new guidelines * WIP: Developer Guidelines * Update Guidelines.md * Update Guidelines.md * Update README * Create Data-Modeling.md * Update Guidelines.md * Update Data-Modeling.md * Update Data-Modeling.md * Update Data-Modeling.md * Update Data-Modeling.md * Update Data-Modeling.md * Update Data-Modeling.md * Update Guidelines.md * Update Guidelines.md * Update Error-Handling.md * Update Error-Handling.md * Update Error-Handling.md * Update Error-Handling.md * Update Error-Handling.md * Update Error-Handling.md * Update Error-Handling.md
1 parent 6abf170 commit 84938b1

11 files changed

+281
-10
lines changed

.github/PULL_REQUEST_TEMPLATE.md

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
## Pull Request (PR) Checklist
22
Please check if your pull request fulfills the following requirements:
33
- [ ] The PR is submitted to the `main` branch.
4-
- [ ] I've read the [Contribution Guidelines](https://github.com/Ivy-Apps/ivy-wallet/blob/main/CONTRIBUTING.md) and my PR doesn't break the rules.
5-
- [ ] I've read the [Architecture Guidelines](https://github.com/Ivy-Apps/ivy-wallet/blob/main/docs/Architecture.md).
4+
- [ ] I've read the [Contribution Guidelines](../CONTRIBUTING.md) and my PR doesn't break the rules.
5+
- [ ] I've read the [Architecture Guidelines](../docs/Guidelines.md).
66
- [ ] I confirm that I've run the code locally and everything works as expected.
77
- [ ] 🎬 I've attached a **screen recoding** of the changes.
88

CONTRIBUTING.md

+1-1
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ _Replace "YOUR_ISSUE_NUMBER" with the id/number of your issue._
4343

4444
### Time to work
4545

46-
Make sure to read the **[Architecture Guidelines 🏗️](docs/Architecture.md)** before you begin.
46+
Make sure to read the **[Developer Guidelines 🏗️](docs/Guidelines.md)** before you begin.
4747

4848
**🔨 Workflow:**
4949

README.md

+9-4
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ Join our Telegram community and drop a message in the "Development" topic.
5858

5959
Ivy Wallet is a great place to code and learn. That's why we're also linking great learning materials (books, articles, videos), check them in **[docs/resources 📚](docs/resources/)**.
6060

61-
Make sure to also check our short **[Architecture Guidelines 🏗️](docs/Architecture.md)** to learn more about the Ivy Wallet's tech side.
61+
Make sure to also check our short **[Developer Guidelines 🏗️](docs/Guidelines.md)** to learn more about the Ivy Wallet's tech side.
6262

6363
[![PRs welcome!](https://img.shields.io/badge/PRs-welcome-brightgreen.svg)](https://github.com/Ivy-Apps/ivy-wallet/blob/main/CONTRIBUTING.md)
6464

@@ -73,22 +73,27 @@ Make sure to also check our short **[Architecture Guidelines 🏗️](docs/Archi
7373
- [Kotlin Flow](https://kotlinlang.org/docs/flow.html) (reactive datastream)
7474
- [Hilt](https://dagger.dev/hilt/) (DI)
7575
- [ArrowKt](https://arrow-kt.io/) (functional programming)
76-
- [Kotest](https://kotest.io/) (unit testing)
76+
77+
78+
### Testing
79+
- [JUnit4](https://github.com/junit-team/junit4) (test framework, compatible with Android)
80+
- [Kotest](https://kotest.io/) (unit test assertions)
81+
- [Paparazzi](https://github.com/cashapp/paparazzi) (screneshot testing)
7782

7883
### Local Persistence
7984
- [DataStore](https://developer.android.com/topic/libraries/architecture/datastore) (key-value storage)
8085
- [Room DB](https://developer.android.com/training/data-storage/room) (SQLite ORM)
8186

8287
### Networking
83-
- [Ktor Client](https://ktor.io/docs/getting-started-ktor-client.html) (REST client)
88+
- [Ktor client](https://ktor.io/docs/getting-started-ktor-client.html) (HTTP client)
8489
- [Kotlinx Serialization](https://github.com/Kotlin/kotlinx.serialization) (JSON serialization)
8590

8691
### Build & CI
8792
- [Gradle KTS](https://docs.gradle.org/current/userguide/kotlin_dsl.html) (Kotlin DSL)
8893
- [Gradle convention plugins](https://docs.gradle.org/current/samples/sample_convention_plugins.html) (build logic)
8994
- [Gradle version catalogs](https://developer.android.com/build/migrate-to-catalogs) (dependencies versions)
90-
- [Fastlane](https://fastlane.tools/) (uploads the app to the Google PlayStore)
9195
- [Github Actions](https://github.com/Ivy-Apps/ivy-wallet/actions) (CI/CD)
96+
- [Fastlane](https://fastlane.tools/) (uploads the app to the Google PlayStore)
9297

9398
### Other
9499
- [Firebase Crashlytics](https://firebase.google.com/products/crashlytics) (stability monitoring)

docs/Guidelines.md

+42
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
# 🏗️ Developer Guidelines
2+
3+
The biggest challenge in software engineering is the fight against **complexity**. If you're planning to contribute to Ivy Wallet, we mainly care about two things:
4+
5+
1. Your PR works and doesn't break anything.
6+
2. Your PR is simple and doesn't add complexity.
7+
8+
Software engineering is also about **thinking**. Don't just follow blindly best practices and strive to do the "right" thing. Instead, ask yourself:
9+
10+
- Is this the simplest solution?
11+
- Am I over-engineering? What does this give me?
12+
- Can we make it simpler?
13+
14+
Be **pragmatic**. Engineering is all about being practical and finding optimal trade-offs. Our world isn't ideal so there isn't a "perfect" solution. The best we can do is optimize for things we care about.
15+
16+
> Follow [the 80/20 principle](https://en.wikipedia.org/wiki/Pareto_principle) - from 20% effort comes 80% of the results.
17+
18+
In Ivy Wallet we care about:
19+
20+
- Software complexity
21+
- Stability
22+
- Simplicity
23+
24+
25+
To recap - **THINK** and keep it simple.
26+
27+
## [Guidelines Wiki](./guidelines)
28+
29+
To get started read:
30+
31+
1. **[Data Modeling](./guidelines/Data-Modeling.md)**
32+
2. **[Error Handling](./guidelines/Error-Handling.md)**
33+
3. **[Unit Testing](./guidelines/Unit-Testing.md)**
34+
4. **[Architecture](./guidelines/Architecture.md)**
35+
5. **[Screen Architecture](./guidelines/Screen-Architecture.md)**
36+
6. **[Screenshot Testing](./guidelines/Screenshot-Testing.md)**
37+
38+
## References
39+
40+
- **[Yoda Style Architecture guidelines](./archive/Yoda-Style-Architecture.md)**
41+
- **[Adnroid Developers - Guide to app architecture](https://developer.android.com/topic/architecture)**
42+
- **[The Grug Brained Developer](https://grugbrain.dev/)**

docs/Architecture.md renamed to docs/archive/Yoda-Style-Architecture.md

+3-3
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ Separation, the path of wisdom it is: **Compose UI ≠ Logic of Screen**. Why? H
2323
4. **Compose Previews:** Mock `UiState`, easy it becomes. Any screen state, preview you can.
2424
5. **Testability:** ViewModel, easy to test it is - send `UiEvent`s, verify `UiState`. Simple. Hmmm... Compose UI, like celebrity is. Mocked `UiState` if you pass, [Paparazzi](https://github.com/cashapp/paparazzi) screenshot tests pass will.
2525

26-
![screen-viewmodel](assets/screen-vm.svg)
26+
![screen-viewmodel](../assets/screen-vm.svg)
2727

2828
### Quick understanding, you seek?
2929

@@ -48,7 +48,7 @@ Reason, straightforward it is. More strength and simplicity, the Compose runtime
4848

4949
Follow [offical Guide to app architecture by Google](https://developer.android.com/topic/architecture), we do. Not for fleeting trends or mere style, but for wisdom it holds. Simplicity, at its core.
5050

51-
![app-architecture](assets/app-layers.svg)
51+
![app-architecture](../assets/app-layers.svg)
5252

5353
### [Data Layer](https://developer.android.com/topic/architecture/data-layer)
5454

@@ -72,7 +72,7 @@ Split our app into many modules, we do. Tangled webs ([spaghetti code](https://e
7272

7373
Each screen, like its own planet it is. Expanding it might, with tales of code and tales. But in its own orbit, it stays. The galaxy of code, peaceful and unshaken it remains.
7474

75-
![modularization-strategy](assets/modularization.svg)
75+
![modularization-strategy](../assets/modularization.svg)
7676

7777
**Simple, our modularization path is:**
7878

docs/guidelines/Architecture.md

Whitespace-only changes.

docs/guidelines/Data-Modeling.md

+120
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,120 @@
1+
# Data Modeling
2+
3+
The way your data is modeled is crucial to the complexity of your software.
4+
We say that a data model `A` is better than a data model `B` <=> `A` eliminates more impossible cases than `B`. Let me show you what that means in practice.
5+
6+
## Algebraic Data Types
7+
8+
**Screen example**
9+
10+
Imagine that you have to implement a screen with three states - loading, success, and error.
11+
A naive way to model its UI state could be:
12+
13+
```kotlin
14+
data class ScreenUiState(
15+
val content: String?,
16+
val loading: Boolean,
17+
val error: String?
18+
)
19+
```
20+
21+
The problem with this approach is that our code will have to deal with many impossible (illegal) states. For example:
22+
23+
- What to show if `loading = false`, `content == null`, `error == null`?
24+
- What to do if we have both `loading = true` and `error != null`?
25+
26+
There are so many ways things to go wrong - for example, a common one is forgetting to reset `loading` back to `false`.
27+
A better way to model this would be to use [Algebraic Data types (ADTs)](https://wiki.haskell.org/Algebraic_data_type)
28+
or simply said in Kotlin: `data classes`, `sealed interfaces`, and combinations of both.
29+
30+
```kotlin
31+
sealed interface ScreenUiState {
32+
data object Loading : ScreenUiState
33+
data class Content(val text: String) : ScreenUiState
34+
data class Error(val msg: String) : ScreenUiState
35+
}
36+
```
37+
38+
With the ADTs representation, we eliminate the impossible cases of having a `content` and `error` at the same time
39+
or `loading = false` and nulls for both `content` and `error`. We also eliminate that on compile-time, meaning that
40+
whatever shit will do - the compiler will never allow this code to run.
41+
42+
**Takeaway:** Model your data using `data classes`, and `sealed interfaces` (and combinations of them) in a way that:
43+
44+
- Mirrors your domain one-to-one, exactly and explicitly.
45+
- Impossible cases are eliminated by construction and at compile-time.
46+
47+
## Explicit Types
48+
49+
Unfortunately, not all impossible cases can be eliminated at compile time. For example, imagine that we're building
50+
a food ordering app and want to model its domain. A naive data model could be:
51+
52+
```kotlin
53+
data class Order(
54+
val id: UUID,
55+
val userId: UUID,
56+
val itemId: UUID,
57+
val count: Int,
58+
val time: LocalDateTime,
59+
val trackingId: String,
60+
)
61+
```
62+
63+
I'm making this up but the goal is to demonstrate common mistakes and how to fix them.
64+
Do you spot them?
65+
66+
Let's think and analyze:
67+
68+
1. What if someone orders a `count = 0` or even worse a `count = -1`?
69+
2. Imagine this function `placeOrder(orderId: UUID, userId: UUID, itemId: UUID, ...)`. How likely is someone to pass a wrong `UUID` and mess UUIDs up?
70+
3. The `trackingId` seems to be required but what if someone passes `trackingId = ""` or `trackingId = "XYZ "`?
71+
72+
I can go on but you see the point. So let's how we can fix it.
73+
74+
```kotlin
75+
data class Order(
76+
val id: OrderId,
77+
val user: UserId,
78+
val item: ItemId,
79+
val count: PositiveInt,
80+
val time: Instant, // <-- always in UTC
81+
val trackingId: NotBlankTrimmedString
82+
)
83+
84+
@JvmInline
85+
value class OrderId(val value: UUID)
86+
87+
@JvmInline
88+
value class UserId(val value: UUID)
89+
90+
@JvmInline
91+
value class ItemId(val value: UUID)
92+
93+
@JvmInline
94+
value class PositiveInt private constructor(val value: Int) {
95+
companion object : Exact<Int, PositiveInt> {
96+
override val exactName = "PositiveInt"
97+
98+
override fun Raise<String>.spec(raw: Int): PositiveInt {
99+
ensure(raw > 0) { "$raw is not >= 0" }
100+
ensure(raw.isFinite()) { "Is not a finite number" }
101+
return PositiveInt(raw)
102+
}
103+
}
104+
}
105+
```
106+
107+
This data model takes more code but you'll thank me for that later because...
108+
**If any of your domain functions accept `order: Order` - you immediately know that it's a valid order and almost no validation logic is required.**
109+
110+
We fixed:
111+
112+
- 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).
113+
- 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` but a function accepts `OrderId`.
114+
- The `time` is now always in UTC by using `Instant`.
115+
- The `trackignId` can't be blank or contain trailing whitespaces.
116+
117+
To learn more about Explicit types you can check [the Arrow Exact GitHub repo](https://github.com/arrow-kt/arrow-exact).
118+
119+
> Not all types can be exact. For example, we make an exception for DTOs and entities where we need primitives.
120+
> However, we still use ADTs and everything in the domain layer where the business logic is must be exact and explicit.

docs/guidelines/Error-Handling.md

+104
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,104 @@
1+
# Error Handling
2+
3+
It's common for operations to fail and we should expect that.
4+
In Ivy Wallet we **do not throw exceptions** but rather make functions that
5+
can fail return [Either<Error, Data>](https://arrow-kt.io/learn/typed-errors/working-with-typed-errors/).
6+
7+
Either is a generic data type that models two possible cases:
8+
- `Either.Left` for the unhappy path (e.g. request failing, invalid input, no network connection)
9+
- `Either.Right` for the happy path
10+
11+
Simplified, `Either` is just:
12+
13+
```kotlin
14+
sealed interface Either<E, A> {
15+
data class Left<E>(val data: E): Either<E, Nothing>
16+
data class Right<T>(val data: A): Either<Nothing, A>
17+
}
18+
19+
fun <E,A,B> Either<E, A>.fold(
20+
mapLeft: (E) -> B,
21+
mapRight: (A) -> B
22+
): B = when(this) {
23+
Either.Left -> mapLeft(data)
24+
Either.Right -> mapRight(data)
25+
}
26+
27+
// a bunch more extension functions and utils
28+
```
29+
30+
So in Ivy, operations that can fail (logically or for some other reason) we'll model using **Either**.
31+
32+
## Data Layer example
33+
34+
Imagine that we're building a program that buys BTC if its price is below $50,000.
35+
36+
```kotlin
37+
interface BtcDataSource {
38+
suspend fun fetchCurrentPriceUSD(): Either<String, PositiveDouble>
39+
suspend buy(amount: PositiveDouble): Either<BuyError, Unit>
40+
41+
sealed interface BuyError {
42+
data class IO(val e: Throwable) : BuyError
43+
data object TooSmallAmount : BuyError
44+
}
45+
}
46+
47+
interface MyBank {
48+
suspend fun currentblBalanceUSD(): Either<Unit, PositiveDouble>
49+
}
50+
51+
class CryptoInvestor @Inject constructor(
52+
private val btcDataSource: BtcDataSource,
53+
private val myBank: MyBank
54+
) {
55+
suspend buyIfCheap(): Either<String, PositiveDouble> = either {
56+
val btcPrice = btcDataSource.fetchCurrentPriceUSD().bind()
57+
// .bind() - if it fails returns Either.Left and short-circuits the function
58+
if(btcPrice.value > 50_000) {
59+
// short-circuits and returns Either.Left with the msg below
60+
raise("BTC is expensive! Won't buy.")
61+
}
62+
val myBalance = myBank.currentBalanceUSD().mapLeft {
63+
"Failed to fetch my bank account balance."
64+
}.bind()
65+
btcDataSource.buy(myBalance).mapLeft { err ->
66+
when(err) {
67+
is BuyError.IO -> "Failed to buy because of an IO error - ${e.msg}"
68+
BuyError.TooSmallAmount -> "Failed to buy because I'm poor."
69+
}
70+
}.bind() // maps the BuyError to String and short-circuits
71+
// Bought BTC with my entire balance!
72+
myBalance // <-- the last line returns the Either.Right
73+
}
74+
}
75+
```
76+
77+
Let's analyze, simplified:
78+
- `either {}` puts us into a "special" scope where the last line returns `Either.Right` and also gives us access to some functions:
79+
- `Operation.bind()`: if the operation fails terminates the `either {}` with operation's `Left` value, otherwise `.bind()` returns the operation's `Right` value
80+
- `raise(E)`: like **throw** but for `either {}` - terminates the function with `Left(E)`
81+
- `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 {}`
82+
83+
## Useful `Either` functions
84+
85+
We won't cover all but I'll list the ones that we often use in Ivy Wallet and find the most useful.
86+
87+
- either {}
88+
- Either.bind()
89+
- raise()
90+
- ensure()
91+
- Either.getOrNull()
92+
- Either.map {} and Either.mapLeft {}
93+
- Either.fold({},{})
94+
95+
I strongly recommend allocating some time to also go through [Arrow's Working with typed errors guide](https://arrow-kt.io/learn/typed-errors/working-with-typed-errors/) which covers much more.
96+
97+
## Fun facts about `Either`
98+
99+
- Either is a [monad](https://en.wikipedia.org/wiki/Monad_(functional_programming)).
100+
- `Either<Throwable, T>` is equivalent to Kotlin's std `Result` type.
101+
- Many projects create a custom `Result<E, T>` while they can just use `Either` with all of its built-in features.
102+
103+
> 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
104+
> (e.g. not enough disk space to write in Room DB / local storage).

docs/guidelines/Screen-Architecture.md

Whitespace-only changes.

docs/guidelines/Screenshot-Testing.md

Whitespace-only changes.

docs/guidelines/Unit-Testing.md

Whitespace-only changes.

0 commit comments

Comments
 (0)