Feedback from emahiro

[emahiro]

フィードバックをつらつら描いていくぞい。

[emahiro]

所感

違和感はある。

interface package is 何？

HTTP を handle するだろう controller と同階層に infra package が存在してることに違和感はある。

entity で単数/複数でファイルを分けてる意図は？

user と users は概念として同じものなのにどうして分けたんだろう？

そもそもこれってちゃんと階層分けされてる？

依存性が内側（上位レイヤー）に向かってるかを一度矢印で考えてみたほうがいいかも。

[emahiro]

個人の見解だが、https://github.com/yuk1ty/easy-learn-clean-architecture/blob/master/iamclean/usecase/userusecase.go#L4-L5 みたいな user2 とかが出てきたら設計が何かおかしいはずだよ。

[yuk1ty]

依存性が内側（上位レイヤー）に向かってるかを一度矢印で考えてみたほうがいいかも。

これはこの設計で正しいと思いますね。

[yuk1ty]

HTTP を handle するだろう controller と同階層に infra package が存在してることに違和感はある。

Clean Architecture では同じレイヤーに属するんで、同じディレクトリに存在していても問題ないです！いくつか流派はありそう。

[yuk1ty]

user と users は概念として同じものなのにどうして分けたんだろう？

これもよくやる手ですが、単体のエンティティ (user) と最終的なアグリゲーション (users) とはわけてエンティティを用意しますね。これは RDS のトランザクションをさばく関係で必要になる実装の想定ですね。

[yuk1ty]

endpoint ← usecase ← repository ← mysql の順になるように階層構造ができているんですが、実際のところは mysql レイヤーは DIP してるので、

endpoint ← usecase ← repository mysql -------------------↑

という構成になっているかと思いますー

[yuk1ty]

個人の見解だが、https://github.com/yuk1ty/easy-learn-clean-architecture/blob/master/iamclean/usecase/userusecase.go#L4-L5 みたいな user2 とかが出てきたら設計が何かおかしいはずだよ。

これが Golang のパッケージのよくわからないところで、package user と entity でも repository でもすると、user 名前空間の中で User も UserRepository も取り出せるかと思いきや、なぜかダブりは禁止で、どちらかの始まりを user2. にしないとダメみたいなんですよね…

[emahiro]

Clean Architecture では同じレイヤーに属する

ここの解釈はいくつか流派がありそう。 オニオンアーキテクチャでいくなら

- (external)interface
  - web
    - api
      - user_api.go
      - request
      - response
  - infra
    - myql
    - redis
    - ...
- usercase
  - xxx_usercase.go
- model 
  - repository
    - user_repository.go
  - entity
    - user.go
    - users.go

でいくかなー。 気になってるのはちゃんとレイヤー分けされてるはずなのに user2 とかいうパッケージが出てきてしまうのは多分何かがミスってるから。そういうのコードに露出するので。

[yuk1ty]

気になってるのはちゃんとレイヤー分けされてるはずなのに user2 とかいうパッケージが出てきてしまうのは多分何かがミスってるから

Golang の名前空間の指定の仕方をミスってるからですね〜…。

Go だと entity や repository の配下ってフラットにしてる例をよく見るんですけど、user, task, category, etc みたいにファイルが増えちゃったときって、そのままにするんですかね？

[yuk1ty]

他の言語だと、

entity

• user
• role
• task
• category ...

といった感じで、欲しい単位でディレクトリを区切ることが多いのですが…

[emahiro]

ちょうどそのパターンを描いていて、domainで分けちゃう方式はある。

- api
  - user
    - handler.go
- infra
  - mysql
  - redis
  - ...
- domain
  - user
    - usecase.go
    - repository.go
    - entity.go
``

[emahiro]

もっと雑にいくならこういうのもあるかと。

- domain
  - user
    - handler.go
    - repository.go
    - usecase.go(service.go)
    - entity.go
- ...

[emahiro]

単体のエンティティ (user) と最終的なアグリゲーション (users) とはわけてエンティティを用意

最終的なアグリゲーションはレスポンスに必要なので Presentation レイヤーで作ってもいいかも。 アグリゲートされた状態で usercase で取り回したい事情があるなら別だけど。

[emahiro]

Golang の名前空間の指定の仕方をミスってるからですね〜…。

package の設計の仕方が usecase が user package に依存する import になってるからでは？

こうなるべきという形は以下かと

package usecase

import(
    "entity"
    "repository"
)

// ....

usecase -> repository (entity) という流れがコードからも明確。

[emahiro]

うまく表現できないが、オニオンアーキテクチャの原則に則るパターンでいくのか、ドメインごとにオニオンアーキテクチャっぽくいくのか、で方針が異なるはず。 豊田の最初のコードはオニオンアーキテクチャに則ってると思ったけど、ドメインで切り分けるアプローチが露出した結果 user2 というのが出てきてしまったのかなと思った（感想）

[yuk1ty]

はやめにレビューしてもらってよかったです。users はちょっとこれから書く処理を見てもらったほうがはやいかもです。パッケージ名はちょっといじってみました！

ドメインで切り分けるアプローチが露出した結果 user2 というのが出てきてしまったのかなと思った

Golang でたぶんこれができないと思いました。

[emahiro]

差分みたし、まぁアリと言えばアリだけどエイリアス切らずに import できるのが綺麗だと思う。

[emahiro]

entity や repository の配下ってフラットにしてる例をよく見るんですけど、user, task, category, etc みたいにファイルが増えちゃったときって、そのままにするんですかね？

そのままにすると思うね。 それか管理しやすくするために階層は分けても pacakge は repository で統一するとか（これも綺麗ではない）

- repository 
  - user
    - user.go

// user.go
pacakege repository

みたいな。

[emahiro]

repository は文字通り repository なので

- repository
  - user.go
  - task.go
  - XXX.go

は普通かと。 さらに最初の違和感に戻るけど entity も eitity package 内でそのままフラットに単体とアグゲートされた結果をフラットに置くことが多いかなと思った。

- entity
  - user.go
  - users.go
  - ...

[emahiro]

なおわしは上記フラットに置くのが違和感あるので domain に分ける方式を自分では使うかも。 但し domain で分ける方針の場合、依存関係がちゃんと流れにそわなくなる時が発生する、というのがあるので、別に service(usecase) をもうけるかなー。

- web
- infra
- service
- domain
  - user
     - entity.go (俺はここにアグリゲーションもセットにするタイプ)
     - repository.go
  - task
     - entity.go
     - repositry.go

api -> service -> domain(repository) という流れ。

各サービスは使うdomainのrepositoryを呼び出す。

// user service は user と task を使って色々ゴニョゴニョする。
type UserService struct {
   userRepo user.UserRepository
   taskRepo task.TaskRepositroy
}

func NewUserService () *UserService{
    return &UserService{
        userRepo: user.NewUserRepository()
        taskRepo: task.NewTaskRepository()
    }
}