お時間のある時に、ソースコードレビューをお願いします。
アーキテクチャの観点からご指摘を頂けると嬉しいです。
ソースの書き方についてもアドバイス頂けたら嬉しいです。
また、アプリケーション層にはどのような役割を持たせたらいいのでしょうか?
僭越ながらご指名頂きましたのでレビューさせて頂きました!
指摘をすべて洗い出しているわけではありませんが、主にアーキテクチャに関する点についてレビュー致しました!
全体の感想
- README に画像を載せてるのがよいと思いました!
- Riverpod の使い方を悩んでいるのかな、と感じました!Riverpod の依存性注入(DI) を使うと、テストが容易になったり、コンストラクタでバケツリレーが不要になりますので、是非 Riverpod の使い方をマスターしてほしいです!
- アプリケーション層は自分もまだその必要性を感じていないのでコメントできないです!すみません。。。
レビュー
Riverpod の DI 機能を使って各層の依存関係を整理しましょう
https://github.com/nzmgfdprs/repository_riverpod_mvvm/blob/0df7b5e9847c80853a20084cdb101c6a33a91e6d/lib/data_layer/post_repository.dart#L9
PostRepository
は FirebaseFirestore.instance
に依存しているので、FirebaseFirestore.instance
を Riverpod の DI で外部から注入してあげましょう。PostRepository
のテストコードを書いてみると、そのメリットを実感できると思います。
以下のようにすると、firebaseFirestoreProvider
の中身をモックに差し替えることでテストが可能になります。
また、外から PostRepository
を使えるようにするために、firebasePostRepositoryProvider
も用意しておきます。
final firebaseFirestoreProvider = Provider(
(_) => FirebaseFirestore.instance,
);
// PostRepository(実体) ではなく IPostRepository (インターフェース)を返してあげる
final firebasePostRepositoryProvider = Provider<IPostRepository>(
(ref) => PostRepository(
// FirebaseFirestoreインスタンスを注入する
store: ref.watch(firebaseFirestoreProvider),
),
);
class PostRepository implements IPostRepository {
PostRepository({
required this.store,
});
final FirebaseFirestore store;
///postsコレクション
@override
get postsCol async => store.collection('posts');
プレゼンテーション層が IPostRepository
を使うときは、ref.watch(firebasePostRepositoryProvider)
とすることで IPostRepository
を取得できます。しかし、これでは、プレゼンテーション層がデータ層に依存してしまいます。プレゼンテーション層はドメイン層とアプリケーション層のみに依存するべきです。この問題は Riverpod の overrides を使って以下のように回避できます。
まず、ドメイン層のリポジトリに次のように空の IPostRepository
を返すプロバイダーを定義します。
final postRepositoryProvider = Provider<IPostRepository>(
(_) => throw UnimplementedError(),
);
そして、main()
で、次のように上書きしてあげます。
void main() async {
runApp(
ProviderScope(
overrides: [
postRepositoryProvider
.overrideWithProvider(firebasePostRepositoryProvider),
],
child: const MyApp(),
),
);
}
こうすることで、プレゼンテーション層が IPostRepository
を使うときは、ドメイン層の postRepositoryProvider
を使って IPostRepository
を取得できるようになり、データ層への依存が無くなります。
https://github.com/nzmgfdprs/repository_riverpod_mvvm/blob/0df7b5e9847c80853a20084cdb101c6a33a91e6d/lib/presentation_layer/view_models/post_page_view_model.dart#L21
PostPageViewModel
は IPostRepository
に依存しているので、コンストラクタで外部から受け取るこのコードは良いと思います!外から PostPageViewModel
を使えるようにするために、postPageViewModelProvider
を用意してあげましょう。その際、postRepositoryProvider
を使って IPostRepository
を取得して注入してあげます。
final postPageViewModelProvider = Provider(
(ref) => PostPageViewModel(
ref.watch(postRepositoryProvider),
),
);
https://github.com/nzmgfdprs/repository_riverpod_mvvm/blob/0df7b5e9847c80853a20084cdb101c6a33a91e6d/lib/presentation_layer/views/post_page.dart#L6-L28
PostPage
内で PostPageViewModel
を使いたいときは、次のようにダイレクトに取得できるようになります。かなりすっきり書けますね!ViewModelをメンバ変数に保持する必要も無いので Stateless の ConsumerWidget
でいけます。
class PostPage extends ConsumerWidget {
const PostPage({Key? key}) : super(key: key);
@override
Widget build(BuildContext context, WidgetRef ref) {
final vm = ref.watch(postPageViewModelProvider);
return Scaffold(
https://github.com/nzmgfdprs/repository_riverpod_mvvm/blob/0df7b5e9847c80853a20084cdb101c6a33a91e6d/lib/presentation_layer/view_models/time_line_page_view_model.dart#L42-L48
PostPage
はコンストラクタに何も受け取らないので、以下のように簡素に書けます。
Future<void> onPost(BuildContext context) async {
Navigator.push(
context,
MaterialPageRoute(
builder: (context) => const PostPage(),
),
);
}
データ層のモデルとドメイン層のモデルを分けた方が良いと思います
https://github.com/nzmgfdprs/repository_riverpod_mvvm/blob/0df7b5e9847c80853a20084cdb101c6a33a91e6d/lib/domain_layer/models/post.dart#L8-L15
JsonKey
があることから、データ層のリポジトリでFirestoreから受け取ったJSONデータを直接ドメイン層のエンティティ(このGitHubリポジトリ上ではモデルと呼ばれている)に変換する意図を感じます。ドメイン層で定義するエンティティは、どの層にも依存しないのがよいので(データ層都合で変更がおきたときに、ドメイン層まで変更が影響してしまう)、直接変換するのではなく、一度データ層用のモデルを挟むのがよいと思います。今回のアプリは小さいのでこれで問題なく動くと思いますが。。
こんなイメージです。
データ層のリポジトリで、Firestoreから受け取ったデータ(Map)を、データ層用のクラス(PostDocument)にfreezedのfromJsonを使って変換する。次に、PostDocumentをドメイン層のエンティティであるPostクラスに変換する。データ層のリポジトリはPostクラスを返す。
https://github.com/susatthi/github-search/blob/32c121e05a1681a9885b5b292db208559d746847/lib/infrastructure/github/repo/repo_repository.dart#L59-L66
同じ事を私は上記でやってます。data(Map)をjsonObject(データ層用のクラス)に変換し、jsonObjectをSearchReposResult(ドメイン層のエンティティ)に変換して返しています。
おすすめのパッケージ