💙
Flutter 研修課題のレビュー観点表
Get Notion free
💙 Page icon💙 Page icon

Flutter 研修課題のレビュー観点表

Flutter 研修課題テンプレート

⚠️ Callout icon
観点表の「適切」という言葉について
具体的に適切かという点はそれぞれの実装によってことなるため、リード・テックリードがサポートする必要がある場合があるかと思います。
しかし評価観点に記してあることに注意することでそれらを学ぶことができれば十分であると考えています。
プログラミングには、問題の解答集のように答えが定まっているわけではありません。
そのため、プログラムのレビューにおいては、曖昧な箇所が出てしまうことがあります。
ある程度レビュー観点をリストアップすることで、より良いプログラムを作成するためのヒントを得ることができます。
レビューにおいて曖昧な箇所があった場合は、リードやテックリードに相談することで、より適切な対応を行うことができます。

研修評価項目

Session 毎のレビュー項目

AspectRatio を使用しているか
1
nits
より意図が分かりやすくなり可読性が向上する
MediaQuery の代わりに FractionallySizedBox を使用しているか
1
nits
const
をつけることができ、パフォーマンスがより向上する
Expanded と Flexible を使い分けているか
1
nits
より適切なものを使うことによって、可読性が向上する
縦画面固定を適切に設定しているか
1
good
縦画面固定は必須ではなく、対応していた場合のレビュー項目
SystemChrome.setPreferredOrientations
 の方法では制限事項があり、特定の条件では機能しない ref: https://gist.github.com/blendthink/7529efc701be36b9bd1f3a9b55570225
Column を余分に利用していないか
1
nits
Theme を適宜利用しているか
1
imo
ページ内に補足あり📝
不必要に Container を利用していないか
1
nits
const で定義できる SizedBox や Padding で置き換えられないか確認する
pubspec.yaml に画像を追加する際、ディレクトリで指定しているか
2
must
後々、画像追加する際に都度修正する必要がなくなり、保守性が向上する
YumemiWeather()
を build 関数内でインスタンス化していないか
2
must
不要なインスタンス生成を日頃から行っていると、ループ処理などの重たい処理を行う際にも記述してしまってパフォーマンスに影響を及ぼしてしてしまう恐れがあるため、日頃から無駄なことをしないように意識する
API の使用箇所でエラーハンドリングしているか
2
nits
APIが修正されて不具合が入り込んだ時のことを想定しておくと、より安全性や保守性が向上する
天気の種類をenumで扱っているか
2
nits
分岐処理で
switch
を使用することができ、可読性や保守性が向上する
Enum の
.values.byName()
を使用していないか
2
must
API が意図せずに変更されたり、不具合が入り込んだりした時のことを想定して、API の処理時に
Error
が発生しないようにすることで、より安全性や保守性が向上する
Enum
.values.byName()
でスローされるのは 
ArgumentError
 で、 アプリケーションコードで
Error
をキャッチするのは適切ではないため、
Exception
をスローする or
null
を返す拡張関数を自作する ref: https://dart.dev/effective-dart/usage#error-handling
テストのことを考えて、
YumemiWeather
のインスタンスを DI しているか
2
next
アセットの指定間違い防止を考慮できているか
2
nits
flutter_gen または、独作
build メソッド直下で画面遷移処理を書いていないか
3
must
予期せぬリビルドが生じる可能性があるため 代わりに、initStateに処理を書いているか
sleep を使っていないか
3
must
スレッドをブロックしてしまうから
mounted のチェックをしているか
3
must
非同期処理で UI を操作しようとするときに mounted のチェックをしていないと、 Widget が Widget Tree に マウントされていない可能性があり、エラーが生じることがある
初期画面と天気画面の Widget を分割しているか
3
good
iOS・Android のスワイプバックを考慮しているか
3
good
go_router
 のバージョン 6.5.0 ~ 6.5.4 だと iOS のスワイプバックで戻ってきた時に、再帰処理が実行されないことがあった。それが原因で、初期画面から天気画面に遷移しないことがあったため、実際の案件ではこのような iOS・Android のスワイプでの挙動の差異がないかというところにも気をつける
go_router
のバージョン 6.5.5 で不具合修正済み ref: https://github.com/flutter/packages/pull/3613
mixin 内での「何かしらの処理」を abstract method にしているか
4
must
void afterDisplayLayout()
⚪︎ •
void afterDisplayLayout() {}
×
エラーの内容によって、メッセージを分けているか
5
good
クリックイベントをメソッドとして切り出しているか
5
nits
ネストが深くなるのを防ぎ、可読性・保守性が高まる
Dialog をコンポーネントとして抽出しているか
5
good
可読性・再利用性を高める
エラーハンドリングが公式ドキュメントに従っているか
5
good
non-null 型への型昇格を適宜利用できているか
6
imo
ページ内に補足あり📝
全ての例外ケースを考慮して JSON 変換処理を書けているか
6
must
json_serializable
or
freezed
を利用していたら、不要
コード生成 ファイルの lint 対応ができているか
7
must
ページ内に補足あり📝
build.yaml にデフォルト値のものを記載しないようにしているか
7
must
デフォルト値を記述すると、他のデフォルト値も全て記載しないといけなくなりそうで、メンバーに混乱が生じる
build.yaml で field_rename: snake をつけているか
7
nits
ページ内に補足あり📝
build.yaml で checked: true にしているか
7
good
ページ内に補足あり📝
freezed_annotation
,
freezed
のコミットを一緒にまとめているか
7
nits
2つのパッケージは関連が強いので、コミットをまとめておいた方が良い
適切に責務分割できているか
8
must
imo
ページ内に補足あり📝
テストがしやすい実装・構成になっているか
8
nits
must
imo
ページ内に補足あり📝
.autoDispose
を適切に設定しているか
8
must
特に意図がない限り基本的に状態は保持しつづけるべきではない
ARCHITECTURE.md はほとんど変更が不要な記載方法になっているか
8
nits
• アーキテクチャの説明でなく、実装の説明になっていないか ◦ 具体的なクラス名やファイル名を記載すると、リネームのたびに修正が必要になる
BuildContext などの UI に関連するものが Notifier で使われていないか
8
imo
ページ内に補足あり📝
アーキテクチャ要素やProviderが循環依存していないか
8
must
Provider の依存関係図を表示している場合、依存関係図を自動生成できるようにしているか
8
good
FYI
riverpod_graphを使うことでProviderの依存関係図を自動生成できる https://github.com/rrousselGit/riverpod/tree/master/packages/riverpod_graph
テストしやすい構成にリファクタリングした際に必要に応じて ARCHITECTURE.md などを更新しているか
9
nits
@visibleForTesting を適宜利用できているか
9
must
• @visibleForTesting だけの場合、警告の表示で無視することもできてしまうため、analyzer の error に設定した方がより安全 • ref: https://kanta-mori.netlify.app/p/visiblefortestingとは/
不必要な
group
を作成していないか
9
nits
テスト実行後に ProviderContainer を dispose しているか
9
must
テストダブルを適切に使えているか
9
must
依存先を明確にしてテストダブルを利用しているか
テストダブルを利用した関数を呼び出す際に any を適宜利用しているか
9
nits
失敗ケースのテストも書いているか
9
nits
例外の場合 thenThrow を使用しているか
9
must
テストの description が適切な表現になっているか
9
10
nits
効果的なテストコードか
9
10
nits
must
imo
ページ内に補足あり📝
デバイスサイズ変更の意図をコメントしているか
10
good
デバイスサイズ変更の後処理(tearDown)をしているか
10
nits
コンポーネントで完結するテストに、画面遷移など他の要素を含めていないか
10
must
複数プラットフォームや複数スクリーンサイズをテストする時は TestVariant を活用しているか
10
good
isolateで扱うべき処理について
11
FYI
ページ内に補足あり📝
syncFetchWeather
への変更に伴い、
ARCHITECTURE.md
, テストの修正を行っているか
11
must

基礎的なレビュー項目

final と const を適切に利用しているか
Dart
final = 実行時不変, const = コンパイル時不変(定数)
const をつけ忘れていないか
Dart
基本的に var を使用しない
Dart
可読性・保守性が低下してしまうため スコープが狭く今後も広くなりづらい場合や、final で書くことによって記述量が増えてしまい可読性が低下する場合は使用してもいいかもしれない ただし、そのような場合はほとんどない
プロパティ、メソッドのスコープは可能な限り狭めているか
Dart
スコープは小さくすると安全性が高まって良い
ロギングを使い分けているか
Dart
目的によるが、print() では不十分な場合がある ref: https://rizumita.medium.com/logging-in-dart-dd9c01eb459a
immutable プログラミングを行えているか
Dart
使用していない、関数の引数は (_) となっているか
Dart
条件分岐を複雑化していないか
Dart
if(条件式) { return … } (早期リターン) などでネストが深くなるのを回避する
getter を適宜活用してるか
Dart
単純に return するだけの場合、わざわざ関数化しない
共通部分は変数に切り出せているか
Dart
enum の条件分岐を if-else でやっていないか
Dart
後から enum に要素が追加される場合、if-else で判定すると修正の必要性に気づけない可能性があるため、 switch - case 使う
マジックナンバーを使用していないか
Dart
意味のある数値を何も説明なく使用しない → 意味のわかる命名で定数を定義して利用するのが良い
Error が発生しないように実装しているか
Dart
nullable な変数を適切に利用しているか
Dart
• 通常はコードの変更にも安全に開発できるようにするために、ローカル変数にして型昇格を行ってから利用する • スコープが狭くほとんど変更されない箇所でnullじゃないと確信できるところは、 “!” を使うことを許容する
pubspec.yaml で開発時のみの依存関係かどうかを考慮してパッケージを追加しているか
Dart
コミット粒度が適切か
GitHub
ビルドできる単位で、など
コミット文が適切か
GitHub
Prefix がついていると見やすい 「エラー直した」など具体性が低いものは NG
プルリクエストを提出してレビュアーがレビューし始めた後で、force push を使用していないか
GitHub
マージされたブランチは削除するように設定しているか
GitHub
ずっとブランチを残していると、ブランチを切り替える際に徐々にコストがかかるようになってしまう
README.md を充実させているか
GitHub
アーキテクチャグラフや環境構築の手順が記載されているか
レビュー後に UI が変更された場合、PR 添付の画像も変更されているかどうか
GitHub
自動生成ファイルのコミット状況に応じて README や CI が適切に設定されているか
GitHub
*.freezed.dart・*.g.dart 等
インデントが揃っているか
IDE
End of File が設定されているか
IDE
Typo を防ぐ仕組みがあるか
IDE
VS Code では、settings.json に Code Spell Checker の設定を行う
保守性
品質
安全性
品質
可読性
品質
一貫したルールに沿っているか
アーキテクチャ
Session11