# Pawnies Code review
Hello team ! As promised during our last meeting, we do a small code review of your code base.
We were really impressed by the code you were able to produce. It is clean, consistent and uses a lot of new technologies (Jetpack-Compose and Kotlin), this means that we have not much to say.

With your team, we can see that the code review process is really working and makes the code really really good. The small comments we will do mostly concern chess and missing comments but not on the implementation details.
# General Remark
## Code structure
- The package structure is relatively well organized. We were just wondering if the package `mobile.state` should be moved in `mobile.ui` or not.
## Code style
- You use a lot of Kotlin features, making the core more readable.
- All the conventions you chose to apply at the start of the project make the code very understanble and consistent. Congratulations for that.
- The classes and functions are well commented.
- You could consider using `enum` values for the collection and field names instead of hardcoded string (e.g. `games`, `users`,`whiteId`, `blackid`, ...). By doing this, you avoid magic number(s) and potential mistakes in the code by writing the value again.
# By file
*Only files for which we have a comment are referenced here.*
## Package `mobile.application.authentication`
### `AuthenticatedUser`
- Maybe you can just rename the `firestore` variable into `store` to avoid confusion.
- The parameters of the class are not commented.
## Package `mobile.application.chess`
### `ChessFacade`
- You could consider using `enum` values for the collection instead of hardcoded string (e.g. `games`, `users`,`whiteId`, `blackid`, ...)
- private data class `StoreMatch` not commented.
## Package `mobile.application.chess.engine`
### `Board`
- Missing comment for `@return` parameter on function `set`. Moreover, you could add a comment explaining that if the piece is null, it is deleted from the broad.
### `Color`
- Missing comment for the function `denormalize`.
## Package `mobile.application.chess.engine.rules`
### `Moves`
- (Regarding TODO in line 204) Just to remind you, in the `castling` functions the position that should not be in check are not the same as the one that have to be empty. We advise you to take only the king's direction as an argument, as the rest is either a position known at the start (`kingStart`) or positions that can be calculated from this direction (empty = all positions where the rook passes, check = all positions where the king passes). For us, this seems better than hardcoded positions.
- We also think that should avoid magic number such as `3` line 239 in the code and use them as constant.
## Package `mobile.application.chess.engine.implementation`
### `PersistentGame`
- Missing comment for some `@param` of the constructor.
## Package `mobile.application.social`
- In the `search` method, make sure not to query the database each time a character is entered as this will quickyl make you run out of Firestore resources.
## Package `mobile.infrastructure.persistence.store`
### `Query`
- Missing comment for `@return` parameter on function `limit`.
## Package `mobile.infrastructure.state` and `mobile.infrastructure.ui`
We didn't check in full details how these two packages work since they consist of graphical stuff and it seems that you are more aware about how this works than us.

# Tests
Your tests look very good, kudos for that.

# Conclusion
Your code is very good overall, you have nice abstractions and lots of efforts put in. The documentation and maning are clear and understable, the codestyle is consistent. The app design is very cool. Keep up the good work!