https://zenn.dev/dowanna6/articles/674c923b26ecd9

どうも。株式会社プラハCEO兼エンジニアの松原です。

先日プラハチャレンジの一環で「DDDに基づいてコードを書いてみよう!」という課題のコードレビューをしていた際、こんなコードを見かけました:

class User {
  constructor(private readonly _articleIds: string[], private readonly _name: string) {
    // 何らかの不変条件
  }

  public get name() {
    return this._name
  }
  public get articleIds() {
    return this._articleIds
  }
}

Userインスタンスのpropertyに対してsetterを定義しなければドメインエンティティの不変条件が守られる!というのが狙いだったのですが、配列の扱いに起因する注意ポイントがあったので記事にしてみようと思いました

TL;DR

mutableな配列を気軽に返すと何が起きるのか

今回のコードは「nameもarticleIdsも外からは変えられない」という前提に立っていました。確かにnameは変更不可なのですが:

const user1 = new User([], 'hoge')
user1.name = 'fuga' // これはread-only propertyなので不可

配列はsetterを定義していなくても中身を変更できます:

const user1 = new User([], 'hoge')
user1.articleIds.push('fuga', 'piyo') // これは可

一時的に変数で保持しておいた配列を別のコードで誤って変更してしまう可能性も:

const user1 = new User([], 'hoge')
const articleIds = user1.articleIds

// ...たくさんのコードが書かれている

articleIds.push('fuga') // 意図せずuser1.articleIdsも変わってしまった!

可変配列を返すとuserのpropertyに対して直接的な配列操作(popとかspliceとか)を許容してしまうので、「特定の条件を満たした時のみ配列操作を許可する」といったドメインロジックを強制できず、Userの不変条件が満たされていること(例えばarticleIdsが常に1件以上5件未満である、など)が保証できない状態に陥りがちです

// 呼び出し側からやりたい放題にされてしまい、全くロジックの整合性が取れなくなったuser1
user1.articleIds.push('1', '2' ,'3', '4', '5') // 本当はarticleIdsに5件以上登録させたくなくても、登録されてしまう
user1.articleIds.splice(1) // 勝手に中身を消されてしまう
user1.articleIds.pop() // 本当はarticleIdsに1件以上は登録しておかなければいけないのに、最後の1件も消されてしまった...

対策: 配列のコピーを返す

userの外から直接配列を操作されることをどのように防げば良いのか、見ていきましょう。

まず配列そのものではなく値をコピーした新しい配列を返せば、直接user内の配列を操作される事がなくなるので安心です: