UniRxアンチパターン集

# はじめに

いままでいくつか「UniRxをこう使うといいよ」に着目していろいろ記事を書いてきました。
その結果としてUniRxがかなり一般的に使われるライブラリとなったため個人的にはとても喜ばしいと感じていました。

その一方で、「**無茶苦茶なUniRxの使い方**」が広まってしまっているとも感じています。
というのも「こう使うといいよ」については触れてきたが、「こう使うべきではない」点についてはあまり触れてこなかったためです。

そこで今回は**UniRxにおけるアンチパターン**をいくつか紹介し、その回避方法について説明します。

※ 文中に「`Observable`」という単語がでてきますが、ニュアンスとしては「≒UniRx」だと思って読んでください。

## アンチパターンとは

アンチパターンとは、「一般的に避けるべきといわれる、好ましくない使い方」を指します。
そのパターンを用いることにメリットが無く、むしろデメリットのほうが大きいようなものを指します。
（ハッキリいってしまうと、「**UniRxを用いたクソコード**」です）

なぜそれがアンチパターンなのかを理解し、覚悟を持った上であえて採用する分には何もいうことはありません。
ですが知らずのうちにアンチパターンへ陥ってるような場合は早急に改善が必要です。

# UniRxアンチパターン集

## アンチパターン: Disposeしてない

`Dispose`とは`IDisposable`インタフェースから提供されるメソッドです。
`Dispose`を呼び出すことでオブジェクトが正常終了し、メモリの解放の準備などが実行されます。
これの呼び出しを忘れた場合、メモリリークしてしまったり場合によってはクラッシュの原因になることもあります。
そのため**`IDisposable`なオブジェクトを触る場合は常に`Dispose`もセットで行うことを意識する必要があります。**

UniRxにおいても`IDisposable`なオブジェクトはいくつか存在します。
それらを大まかに分けると次の2つに分類できます。

* `Subscribe`に対する`Dispose`
* ストリームソースに対する`Dispose`

それぞれの`Dispose`方法や、それを忘れた時に何が起きるのかについて解説します。


### `Subscribe`に対する`Dispose`

#### 概要

`Subscribe`とはUniRxにおいて、ストリーム(`Observable`)を購読する時に実行するメソッドです。
この`Subscribe`は`IObservable<T>`インタフェースに、次のように定義されています。

```cs
namespace System
{
    public interface IObservable<out T>
    {
        IDisposable Subscribe(IObserver<T> observer);
    }
}
```

このように、**`Subscribe`の返り値は`IDisposable`**になっています。
これは「`Subscribe`すると`Observable`に対する購読が始めるが、**この購読を中止したい場合はDisposeを呼んでね**」という意味になっています。

#### Disposeを忘れるとどうなるのか

**SubscribeのDisposeを忘れた場合、裏で購読状態がずっと残ってしまいます。**

`Dispose`を忘れていたとしてもすぐには問題にならないことが多く、質が悪いです。
（何かしらのGameObjectを破棄したタイミングで突然エラーが出たり、またはずっと裏で動いたままCPUを無駄に消費し続けたり）

そもそもUniRxは「`Observable`側が`Subscribe`側に対してメッセージを投げる」という仕組みになっています。
そのため「メッセージを受け取る側がもう存在しないのに`Subscribe`が残ったままになっている」という状況になってしまうと、
`NullReferenceException`などの例外が飛んでエラーだらけになってしまったりします。


たとえば、次のような実装を見てみましょう。
`Observable.EveryFixedUpdate`を用いているにもかかわらず、`Dispose`をし忘れているパターンです。

```cs
using UniRx;
using UnityEngine;

namespace AntiPatterns.NoDispose
{
    public class Enemy : MonoBehaviour
    {
        void Start()
        {
            var rigidBody = GetComponent<Rigidbody>();
            
            // 毎フレーム移動する
            Observable
                .EveryFixedUpdate()
                .Subscribe(_ =>
                {
                    // Rigidbodyにアクセス
                    rigidBody.AddForce(Vector3.forward, ForceMode.VelocityChange);
                });
        }

        /// <summary>
        /// 何かに衝突したら消える
        /// </summary>
        private void OnCollisionEnter(Collision _)
        {
            Destroy(gameObject);
        }
    }
}
```

このようにオブジェクトが`Destroy`されたタイミングで例外が出てしまいました。
例外の内容は「`Destroy`されたオブジェクト（Rigidbody）に対してアクセスが発生してしまった」です。

なぜこうなったかというと、「オブジェクトが破棄された際に`Observable.EveryFixedUpdate`の購読を止めなかった」からです。


#### 対策

「`Subscribe`の`Dispose`漏れ」の対策としては**使い終わったタイミングでDisposeを徹底する**です。
ひとまず`MonoBehaviour`上であれば`AddTo(this)`を使えばなんとかなるため、`MonoBehaviour`を使っているなら`AddTo`をつけることを意識するとよいでしょう。

```cs
using UniRx;
using UnityEngine;

namespace AntiPatterns.NoDispose
{
    public class Enemy : MonoBehaviour
    {
        void Start()
        {
            var rigidBody = GetComponent<Rigidbody>();
            
            // 毎フレーム移動する
            Observable
                .EveryFixedUpdate()
                .Subscribe(_ =>
                {
                    // Rigidbodyにアクセス
                    rigidBody.AddForce(Vector3.forward, ForceMode.VelocityChange);
                })
                // AddTo(this)することで、このGameObjectが破棄された時に
                // 自動的にDisposeを呼び出してくれる
                .AddTo(this); 
        }

        /// <summary>
        /// 何かに衝突したら消える
        /// </summary>
        private void OnCollisionEnter(Collision _)
        {
            Destroy(gameObject);
        }
    }
}
```

もし`Subscribe`の呼び出し元が`MonoBehaviour`を使っていない場合は、そのクラスそのものを`IDisposable`にしてしまい、
`Dispose()`されたときに`Subscribe`も同時に`Dispose()`するなどすると良いでしょう。


```cs
using System;
using UniRx;
using UnityEngine;

namespace AntiPatterns.NoDispose
{
    // Pureなクラス上で、何か別クラスのObservableを購読したとした場合
    public sealed class PureClassObserver : IDisposable
    {
        private readonly IDisposable _disposable;

        public PureClassObserver()
        {
            _disposable = Observable.EveryUpdate()
                .Subscribe(_ =>
                {
                    Debug.Log("なにか処理してる");
                });
        }

        // このクラスがDisposeされたら購読も止める
        public void Dispose()
        {
            _disposable.Dispose();
        }
    }
}
```

また、`AddTo`は`CompositeDisposable`や`CancellationToken`に対しても紐付けることができます。
（`CancellationToken`への紐付けは`UniTask`が必要）
まとめて複数の購読を中止したい場合はこちらも活用すると良いでしょう。


```cs
using System;
using System.Threading;
using Cysharp.Threading.Tasks; //いる
using UniRx;
using UnityEngine;

namespace AntiPatterns.NoDispose
{
    public sealed class PureClassObserver : IDisposable
    {
        // CancellationTokenSource
        private readonly CancellationTokenSource _cancellationTokenSource 
            = new CancellationTokenSource();

        public PureClassObserver()
        {
            Observable.EveryUpdate()
                .Subscribe(_ =>
                {
                    Debug.Log("なにか処理してる");
                })
                // CancellationTokenにAddTo
                .AddTo(_cancellationTokenSource.Token);
        }

        // このクラスがDisposeされたら購読も止める
        public void Dispose()
        {
            _cancellationTokenSource.Cancel();
            _cancellationTokenSource.Dispose();
        }
    }
}
```

他にも直接`Dispose`を呼ばずに、`OnCompleted`メッセージを経由して間接的に`Dispose`を実行するといった手法もあったりします。
（`TakeUntilDestroy`を使うなど）


### ストリームソースに対する`Dispose`

`Subscribe`に対する`Dispose`については、`AddTo`を使っている人が多いのでわりとどんなコードでも徹底されている印象はあります。
しかしその一方でストリームソースに対する`Dispose`はケアしていない人が多い印象を受けます。

ストリームソースとは、`Observable`の源流となるオブジェクトを指します。
たとえば次のようなオブジェクトです。

 * `Subject<T>`
 * `ReactiveProperty<T>`
 * `IConnectableObservable<T>`

 これらオブジェクトはすべて`IDisposable`として定義されており、使い終わった時に`Dispose()`を呼び出すことが推奨されます。


```cs
using System;
using System.Threading;
using Cysharp.Threading.Tasks;
using UniRx;
using UnityEngine;

namespace AntiPatterns.NoDispose
{
    /// <summary>
    /// ReactivePropertyのDispose忘れている例
    /// </summary>
    public sealed class TimerProvider : MonoBehaviour
    {
        // どこにもReactivePropertyのDisposeの記述がない
        private readonly ReactiveProperty<int> _timer 
            = new ReactiveProperty<int>(30);
        
        /// <summary>
        /// 現在のタイマー値
        /// </summary>
        public IReadOnlyReactiveProperty<int> CurrentTime => _timer;

        private void Start()
        {
            // 1秒ごとにカウントダウンする
            LoopAsync(this.GetCancellationTokenOnDestroy()).Forget();
        }

        private async UniTaskVoid LoopAsync(CancellationToken token)
        {
            while (!token.IsCancellationRequested)
            {
                if (_timer.Value <= 0) break;
                
                _timer.Value--;
                await UniTask.Delay(TimeSpan.FromSeconds(1));
            }
        }
    }
}
```


#### Disposeを忘れるとどうなるのか

`Observable`側で`Dispose`を実行すると、その`Observable`に対する購読をまとめて`Dispose`することができます。
そのため`Observable`側の`Dispose`を行うことで、購読をすべて止めることができるため、より安全に`Observable`の破棄ができます。

なお`ReactiveProperty`のみ特殊で、これは「`Dispose()`されたタイミングで`OnCompleted`メッセージを発行する」という挙動になっています。
そのため`ReactiveProperty`を使っているならなおさら`Dispose`の扱いには注意する必要があります。

`Observable`側の`Dispose`漏れはそれがただちに致命的な問題に繋がることは多くありません。
せいぜいGCによる回収が遅延する程度で、致命的にメモリリークしたりすることは多くないです。
ですが`IDisposable`なオブジェクトを放置すること自体が**C#の作法としてよろしくない**ので、徹底して`Dispose()`する癖をつけることを推奨します。

#### 対策

`Subject`や`ReactiveProperty`も忘れずに使い終わったタイミングで`Dispose()`するようにしましょう。
実はこれらも`AddTo`が使えるので、`Awake()`や`Start()`あたりで`AddTo`しておいてもOKです。


```cs
using System;
using System.Threading;
using Cysharp.Threading.Tasks;
using UniRx;
using UnityEngine;

namespace AntiPatterns.NoDispose
{
    /// <summary>
    /// Dispose忘れているやつ
    /// </summary>
    public sealed class TimerProvider : MonoBehaviour
    {
        // どこにもReactivePropertyのDisposeの記述がない
        private readonly ReactiveProperty<int> _timer 
            = new ReactiveProperty<int>(30);
        
        /// <summary>
        /// 現在のタイマー値
        /// </summary>
        public IReadOnlyReactiveProperty<int> CurrentTime => _timer;

        private void Awake()
        {
            // OnDestroy()で _timer.Dispose() するのと同義
            _timer.AddTo(this);
        }

        private void Start()
        {
            // 1秒ごとにカウントダウンする
            LoopAsync(this.GetCancellationTokenOnDestroy()).Forget();
        }

        private async UniTaskVoid LoopAsync(CancellationToken token)
        {
            while (!token.IsCancellationRequested)
            {
                if (_timer.Value <= 0) break;
                
                _timer.Value--;
                await UniTask.Delay(TimeSpan.FromSeconds(1));
            }
        }
    }
}
```

## アンチパターン: なんでもUniRxで書こうとする

これは[2022年現在におけるUniRxの使いみち](https://qiita.com/toRisouP/items/af7d32846ab99f493d92)でも解説したのですが、
「UniRxで書くべき部分」と「UniRxを使わないほうがいい部分」を意識しましょうという話です。

詳しくは上記の記事を読んでほしいのですが、ざっくりと**UniRxを使うべきでない部分**を挙げると次になります。

 * 複雑なロジックを無理やりオペレータチェインで書こうとしている
 * 非同期処理をObservableで扱おうとしている


とくに「ロジックを無理やりオペレータチェインで書こうとしている」はUniRx使い始めの方がよく陥る現象です。
「UniRxの万能感を信じて、すべてのコードをUniRxで書けばきっとキレイになる」という状態です。
そもそも**UniRxは決して万能ではなく、むしろその得意不得意をしっかり意識して使わないといけない**ライブラリです。

「せっかくUniRxを導入したのだからすべてをUniRxで書こう」ではなく、「本当にここでUniRxを使うべきなのか」を考えるようにするとよいでしょう。


## アンチパターン: オペレータの使い方がおかしい（副作用を書く）

**※このミスをやらかす初心者が非常に多いです。**

間違ったオペレータの使い方にもいくつかあるのですが、とくに注意してほしいのは次の1点です。

* オペレータ内で副作用を起こしている
 * たとえば、`Where`の中でフィールド変数の`List<T>`に書き込んでたり
 * たとえば、`Select`内で他の`Subject.OnNext`をキックしてたり


これは**絶対にNGな行為**なので、UniRxを使うときはしっかりとこの点を意識するようにしてください。
**オペレータ内で副作用はダメ、ゼッタイ。**

### そもそも「副作用」とは

そもそも「副作用」が何を意味するかわからない方もいるでしょう。
プログラミングにおける「副作用」の意味は、「**関数を実行した時にその関数の外の状態を変化させてしまうこと**」です。
もっと簡単にいうと、「処理の途中で外の変数を上書きする」といった処理を指します。


たとえば次のような`Add`関数があったとします。
これはうけた引数を用いて計算して、その結果を返すだけなので副作用はありません。

```cs
/// <summary>
/// 足し算する関数
/// これは副作用なし
/// </summary>
public int Add(int x, int y)
{
    return x + y;
}
```

一方で、次のような`Add2`関数があったとします。
これは関数の挙動が外部変数に依存しており、関数を実行するたびに`_sum`の値が変動します。
そのためこの関数は**同じ引数でも呼び出すたびに異なる結果を返す**ことになります。

```cs
/// <summary>
/// 合計値を保持するフィールド変数
/// </summary>
private int _sum = 0;

/// <summary>
/// 足し算する関数
/// 外部変数の状態を書き換えるので副作用あり
/// </summary>
public int Add2(int a)
{
    _sum += a;
    return _sum;
}
```

そもそも「副作用」は**オブジェクト指向プログラミングにおいて避けて通れないもの**です。
「オブジェクト」に「状態」を内包させ「メソッド」経由で副作用を起こしてオブジェクトの状態を変化させる、がオブジェクト指向の基本的な考え方です。
そのため副作用そのものが悪いわけではありません。

「**副作用はそれが許される場所と許されない場所がある**」ということです。

### オペレータ内で副作用がなぜ許容されないか

たとえば、次のような副作用を含んだUniRxのコードがあったとします。
とくに`Where`の使い方に注目してください。

```cs
using System;
using UniRx;
using UniRx.Triggers;
using UnityEngine;

namespace AntiPatterns.SideEffects
{
    public sealed class Player : MonoBehaviour
    {
        private Enemy _lastHitEnemy;

        private void Start()
        {
            // 衝突判定
            this.OnCollisionEnterAsObservable()
                // 敵にぶつかったか判定し、敵だったらフィールドに保存しておく
                .Where(x =>
                {
                    if (x.gameObject.TryGetComponent<Enemy>(out var e))
                    {
                        // フィールドに最後にぶつかった敵を記憶する
                        _lastHitEnemy = e;
                        return true;
                    }

                    return false;
                })
                // 衝突したら1秒間無敵
                .ThrottleFirst(TimeSpan.FromSeconds(1))
                .Subscribe(x =>
                {
                    // ぶつかった場所にエフェクト再生
                    PlayHitEffect(x.contacts[0].point);
                })
                .AddTo(this);
        }

        /// <summary>
        /// 指定座標にエフェクトを再生する
        /// </summary>
        private void PlayHitEffect(Vector3 point)
        {
            // 省略
        }
    }
}
```

`Where`の中で外部の変数`_lastHitEnemy`を上書きする処理を実行しています。
これは紛れもなく「副作用」であり、NGな使い方です。


オペレータの処理内に副作用を混ぜてしまうとそのオペレータのもつ責務が曖昧となり、可読性が著しく落ちてしまいます。

本来`Where`オペレータは「`OnNext`メッセージを条件にしたってフィルタリングする」という責務をもたせるために用います。
これは「コードを書いた人間」と「そのコードを読む人間」とで共通認識であるべきです。
あとから他人がコードを読んだ時に「`Where`を使ってるということは、ここは**条件判定だけ**してるんだな」と、`Where`という名前を見ただけで一瞬で判断できる状態になっているべきです。

同様に他のオペレータにもそれぞれ応じた責務があります。

「`Select`ということは、ここで型変換するんだな」
「`Delay`ということは、ここで遅延を挟むんだな」
「`Take(1)`ということは、1回だけ処理を走らせるつもりなんだな」

といったように、**オペレータ名を見るだけで中身をちゃんと読まなくても全体の流れがすぐ把握できるようになっている**ことが可読性を保つ上で重要な点です。

たとえばさきほどのこのコードですが。

```cs
// 衝突判定
this.OnCollisionEnterAsObservable()
    // 敵にぶつかったか判定し、敵だったらフィールドに保存しておく
    .Where(x =>
    {
        if (x.gameObject.TryGetComponent<Enemy>(out var e))
        {
            // フィールドに最後にぶつかった敵を記憶する
            _lastHitEnemy = e;
            return true;
        }

        return false;
    })
    // 衝突したら1秒間無敵
    .ThrottleFirst(TimeSpan.FromSeconds(1))
    .Subscribe(x =>
    {
        // ぶつかった場所にエフェクト再生
        PlayHitEffect(x.contacts[0].point);
    })
    .AddTo(this);
```

まだラムダ式で`Where`内に直接書いてある分には副作用が隠れていることにまだ気づけます。

これがもし、`Where`の中身が別のメソッドに切り出してあったらどうなるでしょうか。

```cs
private void Start()
{
    // 衝突判定
    this.OnCollisionEnterAsObservable()
        // 敵にぶつかったか判定し、敵だったらフィールドに保存しておく
        .Where(CheckEnemy)
        // 衝突したら1秒間無敵
        .ThrottleFirst(TimeSpan.FromSeconds(1))
        .Subscribe(x =>
        {
            // ぶつかった場所にエフェクト再生
            PlayHitEffect(x.contacts[0].point);
        })
        .AddTo(this);
}

private bool CheckEnemy(Collision x)
{
    if (x.gameObject.TryGetComponent<Enemy>(out var e))
    {
        // フィールドに最後にぶつかった敵を記憶する
        _lastHitEnemy = e;
        return true;
    }

    return false;
}
```

このコードを読んだ時、「`Where`実行時に副作用がある」とすぐ気付けるでしょうか。
それぞれのオペレータが呼び出しているメソッドをすべて精査して副作用が隠れていないかを確認しないとこのコードの挙動を完全に把握することができなくなります。
今回は呼び出してるメソッドが`CheckEnemy()`だけですが、もっと複雑なコードで呼び出しているメソッドが多岐に渡る場合、把握はより一層困難なものとなります。

そしてこのコードの把握を困難にしている最大の原因は「オペレータ内で副作用を起こしている」からです。
そのため**オペレータ内での副作用を禁止すれば困難さは勝手に解消されます**。

もう一度いいます。**オペレータ内に副作用を書くな。マジ。**


### 副作用との付き合い方

実装する上でどうしても副作用が必要になるケースはありえます。
そういった場合、副作用との付き合い方は2つあります。

* 副作用は`Subscribe()`の中のみに限定する
* どうしようもないときの最終手段として`Do()`を使う

#### 副作用との付き合い方:Subscribeの中のみに限定する

とくにひねったような解決策でもなく、「副作用は`Subscribe()`の中まで我慢しろ」という話です。
たとえば、さきほどの副作用を含んでいたコードも次のように書き直すことができます。

```cs
using System;
using UniRx;
using UniRx.Triggers;
using UnityEngine;

namespace AntiPatterns.SideEffects
{
    public sealed class Player : MonoBehaviour
    {
        private Enemy _lastHitEnemy;

        private void Start()
        {
            // 衝突判定
            this.OnCollisionEnterAsObservable()
                // 型を変換
                .Select(x => (Collision: x, Enemy: x.gameObject.GetComponent<Enemy>()))
                // 衝突相手がEnemyだったか
                .Where(t => t.Enemy != null)
                // 衝突したら1秒間無敵
                .ThrottleFirst(TimeSpan.FromSeconds(1))
                .Subscribe(t =>
                {
                    var (collision, enemy) = t;

                    // 最後にぶつかった敵を保存（副作用）
                    _lastHitEnemy = enemy;
                    
                    // ぶつかった場所にエフェクト再生
                    PlayHitEffect(collision.contacts[0].point);
                })
                .AddTo(this);
        }

        /// <summary>
        /// 指定座標にエフェクトを再生する
        /// </summary>
        private void PlayHitEffect(Vector3 point)
        {
            // 省略
        }
    }
}
```

`Select` -> `Where` と多段になっているところに若干のコストを感じるかもしれませんが、**この程度でパフォーマンスに影響がでることはありません**。
効果があるのかよくわからない最適化を求めて副作用だらけのコードを書くくらいなら、多少のコストを払って可読性が高いコードを書いたほうが遥かにマシです。


#### 副作用との付き合い方:Doを使う

どうしても`Observable`の途中で副作用が必要になった時、その救済策として`Do()`というオペレータが用意されています。
ですが「`Do()`を使ってれば副作用は起こしまくってOK」では**ありません**。

`Do()`はいわば「マジでどうしようもないときの最終手段」であり、本来使うべきではありません。
**オペレータとして`Do()`が存在はするが、使うべきではない**です。

（デバッグ用途でログを出すなら`Debug()`オペレータ使ったほうが楽です）

## アンチパターン: Doを多用する

**安易に`Do()`を使うな**

さきほど、「副作用と付き合うためには`Do()`を使うしかない」と言いましたが、これは決して**「`Do()`を使ってるなら副作用コードは書いてもOK」という意味ではありません。**
そもそもの副作用を含んだ処理自体を敬遠すべきであり、`Do()`を使ってようが使ってまいが**そもそも副作用を含んだコードをUniRxで書くな**です。

たとえば次の`Do()`を使ってがんばってる次のコード。
やりたいことはわかるし、そうせざるを得なかった気持ちもわかります。

```cs
using System;
using UniRx;
using UniRx.Triggers;
using UnityEngine;

namespace AntiPatterns.DoWoTsukauna
{
    public sealed class Player : MonoBehaviour
    {
        /// <summary>
        /// 無敵フラグ
        /// </summary>
        public bool IsInvincible { get; private set; }

        private void Start()
        {
            // 敵との衝突処理
            // Doを使ってるあまり良いとは言えないコード
            this.OnCollisionEnterAsObservable()
                // 無敵状態じゃない
                .Where(_ => !IsInvincible)
                // ぶつかった相手が敵
                .Where(x => x.gameObject.TryGetComponent<Enemy>(out _))
                // 無敵状態にしてダメージ処理を実行
                .Do(_ =>
                {
                    IsInvincible = true;
                    OnDamage();
                })
                // 3秒無敵
                .Delay(TimeSpan.FromSeconds(3))
                // 無敵解除
                .Subscribe(_ => IsInvincible = false)
                .AddTo(this);
        }

        /// <summary>
        /// ダメージを受けた処理
        /// </summary>
        private void OnDamage()
        {
            // なんかする
        }
    }
}
```

`Observable`の途中で副作用を起こして、さらにそれを`Delay`などで遅延させて、継続して別の副作用を起こすというもの。
たしかに実装していてこういうパターンは出てきますし、`Do()`を使えばスマートに書けた気持ちになれます。

ですが「そもそもこれ別に`Do()`使わなくても書けるんじゃないか」という観点で一度見て頂きたいです。
`Do()`を使わずにもっとスマートに書けるならそれに越したことはないです。

### Do()を回避する方法

#### 回避策1:`Subscribe()`に書く

`Observable`の途中に`Do()`を挟んでそこに副作用コードを書くことはNGです。
しかし末端部分である`Subscribe()`内であれば副作用コードは許容されるので、副作用をここに寄せてしまうやり方です。


```cs
using System;
using UniRx;
using UniRx.Triggers;
using UnityEngine;

namespace AntiPatterns.DoWoTsukauna
{
    public sealed class Player : MonoBehaviour
    {
        /// <summary>
        /// 無敵フラグ
        /// </summary>
        public bool IsInvincible { get; private set; }

        private void Start()
        {
            // 敵との衝突処理
            // Doを使ってるあまり良いとは言えないコード
            this.OnCollisionEnterAsObservable()
                // 無敵状態じゃない
                .Where(_ => !IsInvincible)
                // ぶつかった相手が敵
                .Where(x => x.gameObject.TryGetComponent<Enemy>(out _))
                .Subscribe(_ =>
                {
                    // 無敵状態にしてダメージ処理を実行
                    IsInvincible = true;
                    OnDamage();

                    // 3秒後に無敵フラグ解除
                    Observable.Timer(TimeSpan.FromSeconds(3))
                        .TakeUntilDestroy(this)
                        .Subscribe(_ =>
                        {
                            IsInvincible = false;
                        });
                })
                .AddTo(this);
        }

        /// <summary>
        /// ダメージを受けた処理
        /// </summary>
        private void OnDamage()
        {
            // なんかする
        }
    }
}
```

`Subscribe()`内で別の`Observable`を`Subscribe()`するのは問題ありません。
`Do()`を使って無理やり1本の`Observable`にするよりかは、多少ダサくても多重`Subscribe()`になっていた方が見通しはよいです。

（それでも`Do()`の方がスマートだ！と思うのなら止めはしませんが…）

#### 回避策2:`async/await`で書く

そもそも副作用が入り乱れるコードを`Observable`で扱うことが無理であったりします。
そういう場合は`UniTask`と`async/await`で手続き的に書いてしまうのも有効です。

```cs
using System;
using System.Threading;
using Cysharp.Threading.Tasks;
using Cysharp.Threading.Tasks.Triggers;
using UnityEngine;

namespace AntiPatterns.DoWoTsukauna
{
    public sealed class Player : MonoBehaviour
    {
        /// <summary>
        /// 無敵フラグ
        /// </summary>
        public bool IsInvincible;

        private void Start()
        {
            // チェックループ起動
            CheckEnemyCollisionAsync(this.GetCancellationTokenOnDestroy()).Forget();
        }

        // 衝突チェックループ
        private async UniTaskVoid CheckEnemyCollisionAsync(CancellationToken token)
        {
            // 衝突判定取得用のAsyncHandler
            var handler = this.GetAsyncCollisionEnterTrigger()
                .GetOnCollisionEnterAsyncHandler(token);

            while (!token.IsCancellationRequested)
            {
                // 衝突待機
                var hit = await handler.OnCollisionEnterAsync();

                // 無敵状態ならスルー
                if (IsInvincible) continue;

                // Enemyじゃないならスルー
                if (!hit.gameObject.TryGetComponent<Enemy>(out _)) continue;

                // 無敵状態にしてダメージ処理を実行
                IsInvincible = true;
                OnDamage();

                // 3秒間無敵維持
                await UniTask.Delay(TimeSpan.FromSeconds(3), cancellationToken: token);

                // 無敵解除
                IsInvincible = false;
            }
        }


        /// <summary>
        /// ダメージを受けた処理
        /// </summary>
        private void OnDamage()
        {
            // なんかする
        }
    }
}
```

ちなみに、このコードは`await foreach`が使える環境ならこっちのほうがキレイに書けます。

```cs
using System;
using System.Threading;
using Cysharp.Threading.Tasks;
using Cysharp.Threading.Tasks.Triggers;
using UnityEngine;

namespace AntiPatterns.DoWoTsukauna
{
    public sealed class Player : MonoBehaviour
    {
        /// <summary>
        /// 無敵フラグ
        /// </summary>
        public bool IsInvincible { get; private set; }

        private void Start()
        {
            // チェックループ起動
            CheckEnemyCollisionAsync(this.GetCancellationTokenOnDestroy()).Forget();
        }

        // 衝突チェックループ
        private async UniTaskVoid CheckEnemyCollisionAsync(CancellationToken token)
        {
            // 衝突イベントをUniTaskAsyncEnumerableとして取得
            await foreach (var hit in this.GetAsyncCollisionEnterTrigger())
            {
                token.ThrowIfCancellationRequested();

                // 無敵状態ならスルー
                if (IsInvincible) continue;

                // Enemyじゃないならスルー
                if (!hit.gameObject.TryGetComponent<Enemy>(out _)) continue;

                // 無敵状態にしてダメージ処理を実行
                IsInvincible = true;
                OnDamage();

                // 3秒間無敵維持
                await UniTask.Delay(TimeSpan.FromSeconds(3), cancellationToken: token);

                // 無敵解除
                IsInvincible = false;
            }
        }

        /// <summary>
        /// ダメージを受けた処理
        /// </summary>
        private void OnDamage()
        {
            // なんかする
        }
    }
}
```

---

「そもそも購読したい対象が最初から`Observable`なんだが」というパターンもあるでしょう。
そういう場合も`Observable`から`UniTask`に変換して、そこから`await`してしまえばOKです。

```cs
using System;
using System.Threading;
using Cysharp.Threading.Tasks;
using UniRx;
using UnityEngine;

namespace AntiPatterns.DoWoTsukauna
{
    /// <summary>
    /// 敵
    /// 10秒ごとに1秒間無敵になる
    /// </summary>
    public class Enemy : MonoBehaviour
    {
        /// <summary>
        /// 無敵フラグ
        /// </summary>
        public bool IsInvincible { get; private set; }

        [SerializeField] private TimerProvider _timerProvider;


        private void Start()
        {
            var token = this.GetCancellationTokenOnDestroy();
            CheckLoopAsync(token).Forget();
        }

        private async UniTaskVoid CheckLoopAsync(CancellationToken token)
        {
            // 10秒ごとに1秒無敵になる
            // （よくわからん挙動だが、まぁ例ということで…）

            while (!token.IsCancellationRequested)
            {
                // _timerProvider.CurrentTime は ReactiveProperty(Observable)
                // ObservableをUniTaskに変換してawait
                await _timerProvider.CurrentTime
                    // 末尾ゼロ秒のときのみ
                    .Where(x => x % 10 == 0)
                    // Observable -> UniTask
                    .ToUniTask(useFirstValue: true, token);

                // 無敵化
                IsInvincible = true;
                // 1秒待機
                await UniTask.Delay(TimeSpan.FromSeconds(1), cancellationToken: token);
                // 無敵解除
                IsInvincible = false;
            }
        }
    }
}
```



## アンチパターン: イベントフローが整理されていない


「なぜ`Observable`を使っているのか書いた本人が理解できてない」というパターンです。
`Observable`を使うメリットとして「依存関係を整理してイベントフローを一方通行にできる」というものがあります。

* [参考](https://qiita.com/toRisouP/items/af7d32846ab99f493d92#f%E4%BE%9D%E5%AD%98%E9%96%A2%E4%BF%82%E3%81%AE%E6%95%B4%E7%90%86%E3%81%AB%E4%BD%BF%E3%81%86)

しかし、このようなデータフローに対する理解が浅いまま`Observable`を使うと次のようなコードを書いてしまうことがあります。

 * 普通にメソッドコールすれば十分な場面ですらなぜか`Observable`を経由している
 * 外のクラス相手に`Subject<T>`を渡す処理があったりする

たとえば次のコード。
`Player`と`PlayerManager`が、`Observable`を通じて連動していますがそこの定義方法が怪しいです。

```cs
using UniRx;
using UnityEngine;

namespace AntiPatterns.TakeObserver
{
    public sealed class PlayerManager : MonoBehaviour
    {
        // PlayerのPrefab
        [SerializeField] private Player _playerPrefab;

        // 今存在しているPlayerの実体
        private Player _currentPlayer;

        private readonly Subject<Unit> _resetSubject = new Subject<Unit>();

        private void Start()
        {
            _resetSubject.AddTo(this);
            CreatePlayer();
        }

        private void OnPlayerDead()
        {
            // Playerが死んだ時の処理がここに
            _currentPlayer = null;

            // 新しいPlayerを生成する
            CreatePlayer();
        }

        private void CreatePlayer()
        {
            // ※ダメな書き方
            
            // Playerを生成
            _currentPlayer = Instantiate(_playerPrefab);

            // 死亡イベント通知用
            var deadSubject = new Subject<Unit>();

            // 通知用のSubjectを渡して初期化
            _currentPlayer.Initialize(deadSubject, _resetSubject);

            // 死亡イベントを取得したら処理を走らせる
            deadSubject.Subscribe(_ => OnPlayerDead());
        }

        // ゲームリセット
        public void GameReset()
        {
            _resetSubject.OnNext(Unit.Default);
        }
    }
}
```

```cs
using System;
using UniRx;
using UnityEngine;

namespace AntiPatterns.TakeObserver
{
    public sealed class Player : MonoBehaviour
    {
        // 死んだ時に通知する用のSubject
        private Subject<Unit> _onDeadSubject;

        /// <summary>
        /// 初期化
        /// </summary>
        public void Initialize(Subject<Unit> deadSubject, IObservable<Unit> resetObservable)
        {
            // 相手から渡されたSubjectを保持しておく
            _onDeadSubject = deadSubject;

            // リセットイベントが来たら位置リセット
            resetObservable
                .Subscribe(_ => Reset())
                .AddTo(this);
        }

        private void Update()
        {
            // 落下したら死亡する
            if (transform.position.y < 0)
            {
                OnDead();
            }
        }

        // リセット処理
        private void Reset()
        {
            // 位置をリセット
            transform.SetPositionAndRotation(Vector3.zero, Quaternion.identity);
        }

        // 死亡処理
        private void OnDead()
        {
            // 死亡結果を相手に通知する
            _onDeadSubject.OnNext(Unit.Default);
            _onDeadSubject.OnCompleted();

            // 破棄
            Destroy(gameObject);
        }
    }
}
```

普通にメソッドコールすればいいものをなぜか無駄に`Observable`経由になってたり、
相手から通知をうける部分もなぜか`Manager`側で`Observable`を用意してたりと**いろいろダメ**なコードです。

### 対策：イベントフローを整理する

じゃあどうすればいいのかですが、一番確実なのは**クラス図を書いて関係を俯瞰してみること**です。

実際、クラス図を書いてみると今のコードはこうなっています。

```plantuml
@startuml
class PlayerManager
{
    - Subject<Unit> resetSubject
    - Subject<Unit> deadSubject
}

class Player
{
    + void Initialize(Subject<Unit> dead,IObservable<Unit> reset)
}


PlayerManager --> Player : "Playerを生成する"
PlayerManager --> Player : "Player"を初期化する
PlayerManager --> Player : 死亡イベントの購読

Player *-- PlayerManager : deadSubjectを保持
Player --> PlayerManager : リセットイベントの購読

@enduml
```

一瞬見て分かるレベルでぐっちゃぐちゃです。
どうしてこうなっているかというと、「**オブジェクトの主従関係がハッキリしていないから**」です。
どちらか一方に参照をよせ、「片方は片方を一方的に知っている」「片方は片方のことを一切知らない」という形にしてあげると関係がスッキリします。

```plantuml
@startuml
class PlayerManager
{

}

class Player
{

}

PlayerManager --> Player
note top of PlayerManager: こういうシンプルな参照関係にもっていきたい

@enduml
```

なので、これを目指してコードを修正します。

```cs
using UniRx;
using UnityEngine;

namespace AntiPatterns.TakeObserver
{
    public sealed class PlayerManager : MonoBehaviour
    {
        // PlayerのPrefab
        [SerializeField] private Player _playerPrefab;

        // 今存在しているPlayerの実体
        private Player _currentPlayer;

        private void Start()
        {
            CreatePlayer();
        }

        private void OnPlayerDead()
        {
            // Playerが死んだ時の処理がここに
            _currentPlayer = null;

            // 新しいPlayerを生成する
            CreatePlayer();
        }

        private void CreatePlayer()
        {
            // Playerを生成
            _currentPlayer = Instantiate(_playerPrefab);

            // 死亡イベントを取得したら処理を走らせる
            _currentPlayer.OnDeadAsync.Subscribe(_ => OnPlayerDead());
        }

        // ゲームリセット
        public void GameReset()
        {
            // 直接メソッド呼び出しすればOK
            _currentPlayer.Reset();
        }
    }
}
```


```cs
using System;
using UniRx;
using UnityEngine;

namespace AntiPatterns.TakeObserver
{
    public sealed class Player : MonoBehaviour
    {
        // 死んだ時に通知する用のSubject
        // Player側にSubject定義もたせたほうがスマート
        private readonly AsyncSubject<Unit> _onDeadSubject = new AsyncSubject<Unit>();

        public IObservable<Unit> OnDeadAsync => _onDeadSubject;
        
        private void Awake()
        {
            _onDeadSubject.AddTo(this);
        }

        private void Update()
        {
            // 落下したら死亡する
            if (transform.position.y < 0)
            {
                OnDead();
            }
        }

        // リセット処理
        public void Reset()
        {
            // 位置をリセット
            transform.SetPositionAndRotation(Vector3.zero, Quaternion.identity);
        }

        // 死亡処理
        private void OnDead()
        {
            // 死亡結果を相手に通知する
            _onDeadSubject.OnNext(Unit.Default);
            _onDeadSubject.OnCompleted();

            // 破棄
            Destroy(gameObject);
        }
    }
}
```

はい、ということで**普通に書いたらこうなるでしょ**、というコードに落ち着きました。

UniRxを使い始めた初心者の人は「何が何でもUniRxで書かないとダメ」という観念にとらわれ、シンプルに書けばいいのに無駄にややこしく書く傾向があったりします。
**UniRxを導入しいてもそれを無理に使う必要はないので、原点に立ち返って素直なコードを書けばOKです。**
（なんでそこまでしてUniRxにこだわるの?という場面が多すぎる）

## アンチパターン: メッセージの発行タイミングが管理しきれていない

これは「イベントフローが整理されていない」に通じるものがあります。
「誰が」「いつ」「どこで」、イベントを発行するかが管理できてないものです。
ひどいパターンだと「`Observable`の途中で`Do()`を使って他の`Subject`にイベントを流し込んでいたり」とかありえます。

こればかりはプロジェクト規模や作ろうとしているものの規模感によるので、対策がムズカシイ部分もあります。
ひとまず、次のことを意識して実装を進めると比較的マシな状況にはできます。


* `Subject<T>`はクラス内にprivateで隠蔽し、外に公開するものは`IObservable<T>`のみにする
* `Subject.OnNext()`を呼び出す場所はできるだけ1つのメソッドに集約する
* イベントの発行に順序関係をもたせる
* `Observable`から他の`Observable`にイベントが飛び火するような入れ子関係をできるだけ避ける
* ファサードパターンを使うなど、そもそも外部モジュールからアクセスするときの口を集約する

# まとめ

* 何がアンチパターンなのか、どうしてアンチパターンといわれるかを理解しよう
* アンチパターンを避けるためにはどうするのか、他の選択肢を選べるようになろう
* UniRxは万能でもなんでもないので、ややこしいと思ったら一切使わないのも手


今回、巷のいろいろなUniRxコードを見て思った感想を書いた記事になってます。
「オイオイ、どういう発想したらそういうコードになるんじゃ？？？？？」というコードを今まで数多く見てきました。
そういった**クソUniRxコード**がなぜクソなのか、の要素を整理した結果が今回の内容になっています。

UniRxは使う機会が減ったとはいえど、まだ現役で使うことはあります。
もしUniRxを使うことがあるならばこのあたりを意識してみるとよいでしょう。