Flutter 研修課題テンプレート
観点表の「適切」という言葉について
具体的に適切かという点はそれぞれの実装によってことなるため、リード・テックリードがサポートする必要がある場合があるかと思います。
しかし評価観点に記してあることに注意することでそれらを学ぶことができれば十分であると考えています。
プログラミングには、問題の解答集のように答えが定まっているわけではありません。
そのため、プログラムのレビューにおいては、曖昧な箇所が出てしまうことがあります。
ある程度レビュー観点をリストアップすることで、より良いプログラムを作成するためのヒントを得ることができます。
レビューにおいて曖昧な箇所があった場合は、リードやテックリードに相談することで、より適切な対応を行うことができます。
研修評価項目
Session 毎のレビュー項目
全体
Session1
Session2
Session3
Session4
Session5
Session6
Session7
Session8
Session9
Session10
Session11
AspectRatio を使用しているか
1
nits
より意図が分かりやすくなり可読性が向上する
MediaQuery の代わりに FractionallySizedBox を使用しているか
1
nits
const
をつけることができ、パフォーマンスがより向上するExpanded と Flexible を使い分けているか
1
nits
より適切なものを使うことによって、可読性が向上する
縦画面固定を適切に設定しているか
1
good
縦画面固定は必須ではなく、対応していた場合のレビュー項目
SystemChrome.setPreferredOrientations
の方法では制限事項があり、特定の条件では機能しない
ref: https://gist.github.com/blendthink/7529efc701be36b9bd1f3a9b55570225Column を余分に利用していないか
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/3613mixin 内での「何かしらの処理」を abstract method にしているか
4
must
•
void afterDisplayLayout()
⚪︎
• void afterDisplayLayout() {}
×エラーの内容によって、メッセージを分けているか
5
good
クリックイベントをメソッドとして切り出しているか
5
nits
ネストが深くなるのを防ぎ、可読性・保守性が高まる
Dialog をコンポーネントとして抽出しているか
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
評価項目
session
レビュータイプ
補足
基礎的なレビュー項目
全体
Dart
Flutter
GitHub
IDE
品質
アーキテクチャ
final と const を適切に利用しているか
Dart
final = 実行時不変, const = コンパイル時不変(定数)
const をつけ忘れていないか
Dart
基本的に var を使用しない
Dart
可読性・保守性が低下してしまうため
スコープが狭く今後も広くなりづらい場合や、final で書くことによって記述量が増えてしまい可読性が低下する場合は使用してもいいかもしれない
ただし、そのような場合はほとんどない
プロパティ、メソッドのスコープは可能な限り狭めているか
Dart
スコープは小さくすると安全性が高まって良い
ロギングを使い分けているか
Dart
目的によるが、print() では不十分な場合がある
ref: https://rizumita.medium.com/logging-in-dart-dd9c01eb459a
使用していない、関数の引数は (_) となっているか
Dart
条件分岐を複雑化していないか
Dart
if(条件式) { return … } (早期リターン) などでネストが深くなるのを回避する
getter を適宜活用してるか
Dart
単純に return するだけの場合、わざわざ関数化しない
共通部分は変数に切り出せているか
Dart
enum の条件分岐を if-else でやっていないか
Dart
後から enum に要素が追加される場合、if-else で判定すると修正の必要性に気づけない可能性があるため、 switch - case 使う
マジックナンバーを使用していないか
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 の設定を行う
保守性
品質
安全性
品質
可読性
品質
一貫したルールに沿っているか
アーキテクチャ
Not Empty29
