質問: 0; i < types.Count; i++) { switch (types[i]) { case Type.炎: //炎のダメージをうけたら燃える break; case Type.斬撃: case Type.打撃: //斬撃、打撃ならバラバラになる break; } } } } ``` この処理の仕方は正しいでしょうか? 独学で勉強して作っているので何が正しくて間違いなのかわかりません。この処理の仕方が正しいのかどうか考えれば考えるほどわからなくなってしまいます。 追記しました 遅くなりすみません。自分のやりたかった通りに動きはしたのですがしばらくしてからもう一度自分の書いたコードを見てみるともっと他に良いやり方があるんじゃないかと考えてしまい何度もコードを書き直しては考え書き直しては考えを繰り返して最終的に結局最初と同じようなコードになったりして全く先に進めません。 今回自分が書いた、相手にダメージを与えて攻撃対象(モンスターや木箱)によって処理を変えて、攻撃に使用した武器に設定されている属性によっても処理を変えたい場合のコードの書き方が今回のやり方で問題はないのかというところで悩んでおり、もっと良いやり方があるとかそのやり方はまずいなどの回答をもらえるとありがたいです。
回答: ※個人的感想が強いので参考までに。もっと良案ありそうだなーとは思います。 「正しいコード」というのは、とどのつまり「動くか動かないか」「高速・低負荷で動作するかどうか」「メンテナンス性が高いかどうか」等が考えられます。 今回のコードは恐らく動くと思いますし速度も極端に遅くなるとは思わないので、「一見綺麗に見えないが、これはこれで最適解かも」と思います。 例えばC#らしくポリモーフィズムに則るなら以下の通り。(正しく則れているかは怪しい) ``` //攻撃の属性クラス(変数は適当) public class AttackType { public string name; public sprite icon; } //攻撃属性クラスを継承した各属性作る public class AttackSlash : AttackType { //斬撃 } public class AttackFire : AttackType { //炎 } //ダメージを受けるオブジェクトの基底クラス //インターフェースではなく抽象クラスにする(MonoBehaviour入れるならここ) public abstract Damageable : MonoBehaviour { public void Damage (float power, AttackSlash type) { //斬撃ダメージ時のデフォ処理 } public void Damage (float power, AttackFire type) { //炎ダメージ時のデフォ処理 } public void Damage (float power, AttackType type) { //それ以外のダメージ時のデフォ処理 } } //武器クラス public class Weapon : MonoBehaviour { public float weaponpower; public AttackType[] types; //インスペクタから入れるなり装備時に入れるなりしておく //攻撃中だけ武器のColliderが有効になる void OnTriggerEnter2D(Collider2D Hitcollider) { //当たったオブジェクトがIDamageableを継承したオブジェクトなら if (Hitcollider.GetComponent() != null) { foreach (var item in types) { Hitcollider.GetComponent().Damage(weaponpower, item); } } } } //敵クラス public class Enemy : Damageable { public void Damage (float power, AttackSlash type) { //斬撃ダメージ時の処理(デフォ処理と違う処理をしたい場合) } //デフォ通りの処理をするならそのメソッドの実装の必要は無い } //木箱クラス public class Kibako : Damageable { public void Damage (float power, AttackSlash type) { //斬撃ダメージ時の処理(デフォ処理と違う処理をしたい場合) } //デフォ通りの処理をするならそのメソッドの実装の必要は無い } ``` この方法の利点は属性が増えた時にDamageableクラスを拡張するだけでいいこと......なのですが、結局個別処理しようと思ったら継承先のメソッドも追記することになるので、今のコードとあんまり変わらないような気もするという。 なので今のコードでもアリかなーと個人的には思います。 (強いて言うなら ・Damageメソッド内のforはWeaponクラスに移したい(同じコードが複数箇所にあるのは極力減らしたい) ・WeaponクラスOnTriggerEnter2D内でnewしているリストはクラス変数へ というぐらいかと)