Switch文でメソッドが長くなりすぎる

Question

###疑問に思うこと
エクセルでモンスターの行動パターンを作ろうと思っています。
行動条件のIdをエクセル内で決めて、プログラム内でそのIdで行動するのかしないのかを判定するメソッドを書いているのですが、Idが多いためSwitch文が長くなってしまいます。

メソッドの行数は長くても30行程度が好ましいと聞いたことがあるのですが、やはり何か工夫をして短くするべきなのでしょうか？
ご指摘お願いします。

public class EN_AI {
	public string Id { get; private set; } //AIの名前.
	public List<EN_AI_Method> AI_Method { get; private set; } //AIの条件パターンのList.
	public EN_AI(string _id, List<EN_AI_Method> _ai_Method) {
		Id = _id;
		AI_Method = _ai_Method;
	}
	
	private bool isUsedOnlyOnce = false;
	private int conditionsValue = 0;
	public ICharaStatus_FVRPG MyStatus { private get; set; }
	public List<ICharaStatus_FVRPG> PlStatusList { private get; set; }
	public List<ICharaStatus_FVRPG> EnStatusList { private get; set; }

	private bool Check_Conditions(string conditionsId) {
		bool result = false;
		switch (conditionsId) {
			case "": result = true; break; //指定なしならtrue.
			case "UNC": result = true; break; //UNC==無条件でtrue.
			case "OnlyOnce": if (isUsedOnlyOnce == false) { isUsedOnlyOnce = true; result = true; } break; //一度きりの行動.
			case "Val_0": if (conditionsValue == 0) { result = true; } break; 
			case "Val_1": if (conditionsValue == 1) { result = true; } break;
			case "Val_2": if (conditionsValue == 2) { result = true; } break;
			case "Val_3": if (conditionsValue == 3) { result = true; } break;
			case "Val_4": if (conditionsValue == 4) { result = true; } break;
			case "Val_5": if (conditionsValue == 5) { result = true; } break;
			case "Val_6": if (conditionsValue == 6) { result = true; } break;
			case "Val_7": if (conditionsValue == 7) { result = true; } break;
			case "Val_8": if (conditionsValue == 8) { result = true; } break;
			case "Rnd_1": if (MyRandom.Judgment_SettingParcent(1.0f)) { result = true; } break; //1%の確立でtrue.
			case "Rnd_3": if (MyRandom.Judgment_SettingParcent(3.0f)) { result = true; } break;
			case "Rnd_5": if (MyRandom.Judgment_SettingParcent(5.0f)) { result = true; } break;
			case "Rnd_10": if (MyRandom.Judgment_SettingParcent(10.0f)) { result = true; } break;
			case "Rnd_20": if (MyRandom.Judgment_SettingParcent(20.0f)) { result = true; } break;
			case "Rnd_30": if (MyRandom.Judgment_SettingParcent(30.0f)) { result = true; } break;
			case "Rnd_50": if (MyRandom.Judgment_SettingParcent(50.0f)) { result = true; } break;
			case "Rnd_80": if (MyRandom.Judgment_SettingParcent(80.0f)) { result = true; } break;
			case "Rnd_99": if (MyRandom.Judgment_SettingParcent(99.0f)) { result = true; } break;
			case "HP_Max": if (MyStatus.Hp_Now == MyStatus.Hp) { result = true; } break;
			case "HP99_&L": if(Calc_HpParcent(MyStatus) <= 0.99f) { result = true; } break; //HP99%以下ならtrue.
			case "HP95_O": if (Calc_HpParcent(MyStatus) > 0.95f) { result = true; } break; //HP95%より上ならtrue.
			case "HP95_&L": if (Calc_HpParcent(MyStatus) <= 0.95f) { result = true; } break;
			case "HP90_O": if (Calc_HpParcent(MyStatus) > 0.9f) { result = true; } break;
			case "HP90_&L": if (Calc_HpParcent(MyStatus) <= 0.90f) { result = true; } break;
			case "HP80_O": if (Calc_HpParcent(MyStatus) > 0.8f) { result = true; } break;
			case "HP80_&L": if (Calc_HpParcent(MyStatus) <= 0.8f) { result = true; } break;
			case "HP70_O": if (Calc_HpParcent(MyStatus) > 0.7f) { result = true; } break;
			case "HP70_&L": if (Calc_HpParcent(MyStatus) <= 0.7f) { result = true; } break;
			case "HP60_O": if (Calc_HpParcent(MyStatus) > 0.6f) { result = true; } break;
			case "HP60_&L": if (Calc_HpParcent(MyStatus) <= 0.6f) { result = true; } break;
			case "HP50_O": if (Calc_HpParcent(MyStatus) > 0.5f) { result = true; } break;
			case "HP50_&L": if (Calc_HpParcent(MyStatus) <= 0.5f) { result = true; } break;
			case "HP40_O": if (Calc_HpParcent(MyStatus) > 0.4f) { result = true; } break;
			case "HP40_&L": if (Calc_HpParcent(MyStatus) <= 0.4f) { result = true; } break;
			case "HP30_O": if (Calc_HpParcent(MyStatus) > 0.3f) { result = true; } break;
			case "HP30_&L": if (Calc_HpParcent(MyStatus) <= 0.3f) { result = true; } break;
			case "HP20_O": if(Calc_HpParcent(MyStatus) > 0.2f) { result = true; } break;
			//以下略。味方の属性が○○か、敵の属性が○○か、○○という名のキャラはいるのかなど、まだまた書かないといけない.
		}
		return result;
	}
	private float Calc_HpParcent(ICharaStatus_FVRPG checkStatus) { return checkStatus.Hp_Now / (float)checkStatus.Hp; }
}

Answer 1

いいハマり方をしていますね。

「デザインパターン」で検索してみてください。
デザインパターンとは、過去の偉人たちが編み出した、「こういうときはこうするといいよ」というバターン集です。
様々なパターンがあり、そのどれもが「なるほど、よく考えたな」と思えるものばかりです。
オブジェクト指向以上に「そんなやり方があったのか！」と思わせてくれると思います。

そして今回の例では、「Stateパターン」などが比較的親和性が高いと思います。

簡単にどんなパターンか説明すると。
それぞれif文で分岐している部分を「状態（State）」と呼びます。
そしてこの「状態」を簡単に、かつメンテナンス性よく解決してくれるのが「Stateパターン」です。

ここからは個人的なイメージになりますが、各Stateは、例えばNintendo 3DSのカートリッジに近いイメージです。
というとなんのこっちゃかもしれませんが、つまり。

Aという状態にいるときはAというカートリッジを挿す。
すると、その状態ではAに定義された処理しかされなくなる、という具合です。
もしBという状態に変化しそうになったらBというカートリッジに差し替える、という具合に状態を変更していきます。
これの何がいいかというと、質問で書かれている状態処理を以下のようにとても簡潔に書くことができるようになります。

anyState.doAnything();

です。
そしてもし、状態が変わる場合は

anyState = new OtherState();

というように、anyState自体を「差し替えて」しまうのです。（これがカートリッジ、と例えた所以です）

こうした状態に応じた処理というのは概ね、「その状態の中ならこうする」と状態ごとに処理をまとめられることがほとんどです。
つまり、上記の例で言えばAnyStateクラスやOtherStateクラスのdoAnythingメソッドに、状態ごとの処理を書いておけば、あとはこの状態を差し替えるだけで完了します。
もし状態を増やしたくなったら、その分だけ新しい状態クラスを追加して差し替えるだけでよくなります。

個人的に、デザインパターンの中で一番好きなパターンがこのStateパターンです。
ぜひ色々「デザインパターン」で調べてみてください。

Comments

[ozwk]

横からなんですが、excelに定義してあるということなので、
stateを生成するときに工夫しないと結局switch文が長くなりそうなんですが。

[edom18]

そうですね。ステートの生成はまた別のロジックが必要だと思います。

[ozwk]

その別のロジックってどうすればいいですかね？

[edom18]

うーん、やりたいことによるので、質問の内容からだとこれ、というのはむずかしいですね。

Answer 2

こういうの作ってみてはどうかな？

public class EN_AI
{
    private static IDictionary<string, Func<EN_AI, bool>> predicates;
    public static Register(string name, Func<EN_AI, bool> predicate)
    {
        predicates[name] = predicate;
    }

    private bool Check_Conditions(string conditionsId)
    {
        Func<EN_AI, bool> predicate;
        if (!predicates.TryGetValue(conditionId, out predicate))
        {
           throw new Exception("判定条件が登録されていません。");
        }

        return predicate(this);
    }
}

これは貰ったサンプルを基にしてるけど、応用で分割したりグルーピングしたりファイルから読み込めるようにしたり…って具合です。ロジック自体はシンプルになると思うけどどうでしょうか。

こんな感じで他から追加できるので、Registerの方法を工夫するようにします。

foreach (var i in Enumerable.Range(0, 8))
{
    int num = i;
    EN_AI.Register($"Val_{num}", ai => ai.ConditionsValue == num);
}

これで条件文と判定の切り分けができます。

Answer 3

デザインパターン云々…でいうと…
自分に思いつくのは今回の要件ではStrategyパターンぐらいなので、応用例を書いておきます。

※私はゲームプログラミングしたことがないので命名はかなり適当です。参考にしないでね

    // 条件を判定する要素の規定インターフェース
    public interface IConditionPredicate
    {
        string ConditionId { get; }

        // paramの型をどうするかは一考の余地がありますが
        bool Check(EN_AI enemy, string[] parameters);
    }

    // 判定条件一つ一つをクラス化して…
    public class ValueConditionPredicate : IConditionPredicate
    {
        public string ConditionId { get; } = "Val";

        public bool Check(EN_AI enemy, string[] parameters)
        {
            int value;
            if (parameters.Length != 1 || !int.TryParse(parameters[0], out value)) return false;
            return enemy.ConditionValue == value;
        }
    }

    public class RandomConditionPredicate : IConditionPredicate
    {
        private readonly Random randomGenerator = new Random(Guid.NewGuid().GetHashCode());

        public string ConditionId { get; } = "Rnd";

        public bool Check(EN_AI enemy, string[] parameters)
        {
            int value;
            if (parameters.Length != 1 || !int.TryParse(parameters[0], out value)) return false;
            return randomGenerator.Next() % 100 < value;
        }
    }

    public class HpConditionPredicate : IConditionPredicate
    {
        public string ConditionId { get; } = "HP";

        public bool Check(EN_AI enemy, string[] parameters)
        {
            if (parameters.Length != 2) return false;

            string function = parameters[0];
            int value;
            if (!int.TryParse(parameters[1], out value)) return false;

            int percent = enemy.Hp.Percent;
            switch (function)
            {
                case "G":
                    return value >= percent;
                case "L":
                    return value <= percent;
                case "E":
                    return value == percent;
            }
            return false;
        }
    }

    // それを元に判定するクラスを作ります。
    public class ConditionChecker
    {
        private IDictionary<string, KeyValuePair<string, string[]>> Replacements;
        private IDictionary<string, IConditionPredicate> predicates;

        // 条件を追加
        public void AddConditionPredicate(IConditionPredicate predicate)
        {
            predicates.Add(predicate.ConditionId, predicate);
        }

        // とあるIDを別のIDに変換するためのマッピングを登録
        public void AddReplacement(string name, string conditionId, string[] parameters)
        {
            Replacements.Add(name, new KeyValuePair<string, string[]>(conditionId, parameters));
        }

        // マッピングされた置換を適用する関数
        private void Adjust(ref string name, ref string[] parameters)
        {
            KeyValuePair<string, string[]> replace;
            if (!Replacements.TryGetValue(name, out replace)) return;

            name = replace.Key;
            parameters = replace.Value;
        }

        // 初期化サンプル
        // こういうのをある程度自動化する方法もあります。
        // 例：IConditionPredicateを実装したクラスをリフレクションでインスタンス化します
        public void InitializingSample()
        {
            AddConditionPredicate(new ValueConditionPredicate());
            AddConditionPredicate(new RandomConditionPredicate());
            AddConditionPredicate(new HpConditionPredicate());

            // ConditionIDがHP_MaxだったらHP100%と一致、という風な書き換え条件を登録
            AddReplacement("HP_Max", "HP", new string[] { "E", "100" });
        }

        // 本題の判定関数
        public bool Check(EN_AI enemy, string conditionId, string[] parameters)
        {
            Adjust(ref conditionId, ref parameters);

            IConditionPredicate predicate;
            if (!predicates.TryGetValue(conditionId, out predicate))
            {
                throw new Exception();
            }

            return predicate.Check(enemy, parameters);
        }
    }

こんな感じでちょっとずつばらしていくと、判定毎に複雑なロジックを実装できます。

上記サンプルはExcelに複数行で条件を登録することを想定しています。
例えばHP80%以上の場合、HP | G | 80 とかそんな風に登録します。
複雑な条件を別の書き方ができるように、HP_Max ってIDをHP | E | 100 に変換したりもしています。

初期化部分を工夫していけば、今後の条件追加やロジック追加が簡単になります。

また、IConditionPredicateそのものを拡張してEN_AIっていうモンスター単体のオブジェクトから、BattleContextのような戦闘全体を把握できるステートも渡せるように改良すれば、例えば「３ターン目に実行する」とか、「味方が死んでいたら」とかそんな条件文も作ることができるようになります。

何にせよ、完璧なカスタマイズ性よりは、まず動くものを作るのが一番大事ですから、こういうのに挑戦するのはまだ先で良いと思います。

※オマケの部分

    public interface ICharaStatus_FVRPG
    {
        int MaxHp { get; set; }
        int Hp { get; set; }
    }

    public class EN_AI
    {
        public int ConditionValue { get; set; }
        public ICharaStatus_FVRPG MyStatus { private get; set; }
        public HpAccessor Hp { get { return new HpAccessor(MyStatus); } }
    }

    public struct HpAccessor
    {
        private readonly ICharaStatus_FVRPG status;
        public HpAccessor(ICharaStatus_FVRPG status)
        {
            this.status = status;
        }

        public int Percent
        {
            get
            {
                return Calc_HpPercent(status);
            }
        }

        private static int Calc_HpPercent(ICharaStatus_FVRPG status)
        {
            return status.Hp * 100 / status.MaxHp;
        }
    }

Comments

[kyoro_san]

丁寧なサンプルコードありがとうございます。
ただ、今の自分のレベルでは難しいので、最後におっしゃっているようにとりあえずちゃんと動くようになってから、他の回答者さんが勧めてくれているデザインパターンや正規表現、上記のサンプルコードを調べながら自分のコードの改善点を探していこうと思います。
ありがとうございました。

Answer 4

はいはい、こんばんは、なんとなく回答してみます。
最適なのはやっぱりStateパターンを使うのが一番かなとは思いますが、問いにあります
「Switch文でメソッドが長くなりすぎる」の回答案を書かせていただきます。
正直switchは使いたくない！と言うのが正直なところなのでif分に出してみました。

それと、処理をコンディションごとにメソッドに吐きだしています。

    private bool Check_Conditions(string conditionsId) {
        bool result = false;
        if (conditionsId == "") result = true;
        if (conditionsId == "UNC") result = true;
        if (conditionsId == "OnlyOnce") if (isUsedOnlyOnce == false) { isUsedOnlyOnce = true; result = true; };

        if(checkVal(conditionsId,conditionsValue))result = true;
        if(checkRnd(conditionsId))result = true;

        if(checkHP(conditionsId))result = true;
        switch (conditionsId) {

            case "HP_Max": if (MyStatus.Hp_Now == MyStatus.Hp) { result = true; } break;
            case "HP99_&L": if(Calc_HpParcent(MyStatus) <= 0.99f) { result = true; } break; //HP99%以下ならtrue.
            case "HP95_O": if (Calc_HpParcent(MyStatus) > 0.95f) { result = true; } break; //HP95%より上ならtrue.
            case "HP95_&L": if (Calc_HpParcent(MyStatus) <= 0.95f) { result = true; } break;
            case "HP90_O": if (Calc_HpParcent(MyStatus) > 0.9f) { result = true; } break;
            case "HP90_&L": if (Calc_HpParcent(MyStatus) <= 0.90f) { result = true; } break;
            case "HP80_O": if (Calc_HpParcent(MyStatus) > 0.8f) { result = true; } break;
            case "HP80_&L": if (Calc_HpParcent(MyStatus) <= 0.8f) { result = true; } break;
            case "HP70_O": if (Calc_HpParcent(MyStatus) > 0.7f) { result = true; } break;
            case "HP70_&L": if (Calc_HpParcent(MyStatus) <= 0.7f) { result = true; } break;
            case "HP60_O": if (Calc_HpParcent(MyStatus) > 0.6f) { result = true; } break;
            case "HP60_&L": if (Calc_HpParcent(MyStatus) <= 0.6f) { result = true; } break;
            case "HP50_O": if (Calc_HpParcent(MyStatus) > 0.5f) { result = true; } break;
            case "HP50_&L": if (Calc_HpParcent(MyStatus) <= 0.5f) { result = true; } break;
            case "HP40_O": if (Calc_HpParcent(MyStatus) > 0.4f) { result = true; } break;
            case "HP40_&L": if (Calc_HpParcent(MyStatus) <= 0.4f) { result = true; } break;
            case "HP30_O": if (Calc_HpParcent(MyStatus) > 0.3f) { result = true; } break;
            case "HP30_&L": if (Calc_HpParcent(MyStatus) <= 0.3f) { result = true; } break;
            case "HP20_O": if(Calc_HpParcent(MyStatus) > 0.2f) { result = true; } break;
            //以下略。味方の属性が○○か、敵の属性が○○か、○○という名のキャラはいるのかなど、まだまた書かないといけない.
        }
        return result;
    }

    private boolen checkVal(string id, int value){
        if (!id.startsWith("Val_")) return false;
        int cValue = id.Substring(4);
        return (cValue == value);
    }
    private boolen checkRnd(string id){
        if (!id.startsWith("Rnd_")) return false;
        float fValue = float.Parse(id.Substring(4));
        return MyRandom.Judgment_SettingParcent(fValue);
    }

例のソースに
result = trueとありましたので、毎度 resultにフラグを立てていますが、もし下で返すだけを考えていらっしゃる場合でしたら

        if (conditionsId == "") return true;

と書いた方がスマートですよ。

HP部分は値の抽出など正規表現を使うと同じように出来ますが、無理せずswitchか、swichを書いたメソッドにはき出すのが一番だと思いますよ。
正規表現 : Regex.Replace メソッド (System.Text.RegularExpressions)

            case "HP80_&L": if (Calc_HpParcent(MyStatus) <= 0.8f) { result = true; } break;

今後を考えると、Stateパターンなど組み込むことなど考えるのも一興かもしれませんね。

19．State パターン | TECHSCORE(テックスコア)

Answer 5

こういうときこそ、オブジェクト指向のポルモアフィズムを使います。
利用言語が書いてないので雰囲気だけですが、

public class Val extends EN_AI {
  int myCondition;
  Val(int c) {
    myCondition = c;
  }
  public bool Check_Conditions() {
    return conditionsValue == myCondition;
  }
}

public class Rnd extends EN_AI {
  ...
}

public class HP extends EN_AI {
  ...
} 

みたいにサブクラス毎に振る舞いを分けて、初期化時にサブクラスを指定して new し、数値パラメータもコンストラクタに与えてはどうでしょうか？ あ、 Check_Condition は抽象メソッドにする必要があります。