# Code review!
Inspire from [google](https://google.github.io/eng-practices/review/reviewer/standard.html)'s guildline.
---
## Session
- The Standard of Code
- Principles
---
## The Standard of Code
本質是讓 codebase 能夠
維持一定的 "`健康度`" 並且是不斷地 "`進步`" 的
The primary purpose of code review is to make sure that the overall code health of our’s code base is **improving** over time.
---
### 主要重點在於「並沒有所謂完美的程式，只有更佳的程式」
---
### Principles
1. 要能給出技術上的建議，而不是個人偏好。
3. 關於軟體設計的部分，必須根據設計的基本原則去權衡考量，而不是基於個人偏好去更動原先的做法。
4. 假使有些部分並沒有前例可以參考時，在不會讓 codebase 變糟前提，應該要 Follow 目前的做法以保持一致性。
[Ref](https://google.github.io/eng-practices/review/reviewer/standard.html)
---
### Resolving Conflicts
- Face-to-face meeting
- Maintainer, tead leader, manager
---
### What to look for in a code review (1/2)
- Design: 整體的設計
- Functionality: 現在，過去，未來
- Complexity: 複雜度，可讀性，over-engineering
- Tests: 在合理的時間允許下，Reviewer 應該要要求寫測試
- Naming: 要能夠清楚表達這是什麼，或者這是在做什麼，以維持可讀性。
- Comment: 這段 Code 為什麼存在，主要是為了達到什麼效果
[Ref](https://tachingchen.com/tw/blog/how-to-do-a-code-review-by-google-2/)
---
### What to look for in a code review (2/2)
- Style: Style guide
- Documentation: 撰寫維護
- Every line: 要能夠理解每行 Code，call out for explaining
- Context: 上下文
- Good things: 有時候如果 Developer 寫得好，也可以留下 Comment 來讚美，因為每次的 Review，其實都可以當作一個 Mentoring 的機會，這也會帶來正面的助益。
[Ref](https://tachingchen.com/tw/blog/how-to-do-a-code-review-by-google-2/)
---
### 1. Naming
#### Before
``` php
$status = $user->status('pending');
```
---
### 1. Naming
#### After
``` php
$isUserPending = $user->isStatus('pending');
```
---
### 2. Naming
#### Before
``` php
return $factory->getTargetClass();
```
---
### 2. Naming
#### After
``` php
return $factory->getTargetClassPath();
```
---
### 3. Extracting
#### Before
``` php
public function setCodeExamples(string $exampleBefore, string $exampleAfter)
{
$this->exampleBefore = file_get_contents(base_path("$exampleBefore.md"));
$this->exampleAfter = file_get_contents(base_path("$exampleAfter.md"));
}
```
---
### 3. Extracting
#### After
``` php
public function setCodeExamples(string $exampleBefore, string $exampleAfter)
{
$this->exampleBefore = $this->getCodeExample($exampleBefore);
$this->exampleAfter = $this->getCodeExample($exampleAfter);
}
private function getCodeExample(string $exampleName): string
{
return file_get_contents(base_path("$exampleName.md"));
}
```
---
### 4. Extracting
#### Before
``` php
User::whereNotNull('subscribed')->where('status', 'active');
```
---
### 4. Extracting
#### After
``` php
User::subscribed();
```
---
### 5. Return Early
#### Before
``` php
public function calculateScore(User $user): int
{
if ($user->inactive) {
$score = 0;
} else {
if ($user->hasBonus) {
$score = $user->score + $this->bonus;
} else {
$score = $user->score;
}
}
return $score;
}
```
---
### 5. Return Early
#### After
``` php
public function calculateScore(User $user): int
{
if ($user->inactive) {
return 0;
}
if ($user->hasBonus) {
return $user->score + $this->bonus;
}
return $user->score;
}
```
---
### 6. Return Early
#### Before
``` php
public function sendInvoice(Invoice $invoice): void
{
if($user->notificationChannel === 'Slack')
{
$this->notifier->slack($invoice);
} else {
$this->notifier->email($invoice);
}
}
```
---
### 6. Return Early
#### After
``` php
public function sendInvoice(Invoice $invoice): bool
{
if($user->notificationChannel === 'Slack')
{
return $this->notifier->slack($invoice);
}
return $this->notifier->email($invoice);
}
```
---
### 7. Use collection
#### Before
``` php
$score = 0;
foreach($this->playedGames as $game) {
$score += $game->score;
}
return $score;
```
---
### 7. Use collection
#### After
``` php
return collect($this->playedGames)
->sum('score');
```
---
### 8. Use collection
#### Before
``` php
$users = [
[ 'id' => 801, 'name' => 'Peter', 'score' => 505, 'active' => true],
[ 'id' => 844, 'name' => 'Mary', 'score' => 704, 'active' => true],
[ 'id' => 542, 'name' => 'Norman', 'score' => 104, 'active' => false],
];
// Requested Result: only active users, sorted by score ["Mary(704)","Peter(505)"]
$users = array_filter($users, fn ($user) => $user['active']);
usort($users, fn($a, $b) => $a['score'] < $b['score']);
$userHighScoreTitles = array_map(fn($user) => $user['name'] . '(' . $user['score'] . ')', $users);
return $userHighScoreTitles;
```
---
### 8. Use collection
#### After
``` php
$users = [
[ 'id' => 801, 'name' => 'Peter', 'score' => 505, 'active' => true],
[ 'id' => 844, 'name' => 'Mary', 'score' => 104, 'active' => true],
[ 'id' => 542, 'name' => 'Norman', 'score' => 104, 'active' => false],
];
// Requested Result: only active users, sorted by score ["Mary(704)","Peter(505)"]
return collect($users)
->filter(fn($user) => $user['active'])
->sortBy('score')
->map(fn($user) => "{$user['name']} ({$user['score']})"
->values()
->toArray();
```
---
### 9. Consistency
#### Before
``` php
class UserController
{
public function find($userId) {}
}
class InvoicesController
{
public function find($user_id) {}
}
```
---
### 9. Consistency
#### After
``` php
class UserController
{
public function find($userId) {}
}
class InvoiceController
{
public function find($userId) {}
}
```
---
### 10. Consistency
#### Before
``` php
class PdfExporter
{
public function handle(Collection $items): void
{
// export items...
}
}
class CsvExporter
{
public function export(Collection $items): void
{
// export items...
}
}
$pdfExport->handle();
$csvExporter->export();
```
---
### 10. Consistency
#### After
``` php
interface Exporter
{
public function export(Collection $items): void;
}
class PdfExporter implements Exporter
{
public function export(Collection $items): void
{
// export items...
}
}
class CsvExporter implements Exporter
{
public function export(Collection $items): void
{
// export items...
}
}
$pdfExport->export();
$csvExporter->export();
```
---
### Reference
- https://google.github.io/eng-practices/review/reviewer/looking-for.html
- https://medium.com/@ryanyang1221/%E8%AE%93-google-%E6%95%99%E4%BD%A0-code-review-be251d4d81b4
- https://tachingchen.com/tw/blog/how-to-do-a-code-review-by-google-1
{"metaMigratedAt":"2023-06-15T13:11:17.951Z","metaMigratedFrom":"YAML","title":"How to do a code review","breaks":true,"description":"View the slide with \"Slide Mode\".","contributors":"[{\"id\":\"73654cf3-8e85-4412-809c-0d74345d349e\",\"add\":10596,\"del\":6355}]"}