【Unity】コード保守性の向上術：保守性が低いコードと高いコードの違い

2024年5月23日

保守性の高いコードは、変更や拡張が容易で、バグの修正が簡単なコードです。具体的には、読みやすい名前付け、明確なコメント、モジュール化された構造、テストの充実などが特徴です。一方、保守性の低いコードは、複雑で理解しにくく、変更が困難です。グローバル変数の多用や重複コード、コメント不足、スパゲッティコードなどがその例です。保守性を高めるためには、設計の段階から注意し、コードレビューやリファクタリングを定期的に行うことが重要です。

サンプル

保守性が低いコード

using UnityEngine;

public class Player : MonoBehaviour
{
    void Update()
    {
        if (Input.GetKeyDown(KeyCode.Space))
        {
            GetComponent<Rigidbody2D>().AddForce(new Vector2(0, 10), ForceMode2D.Impulse);
        }

        if (transform.position.y > 5)
        {
            GetComponent<SpriteRenderer>().color = Color.red;
        }
    }
}

問題点

1. ハードコーディングされた値が多い（例：new Vector2(0, 10), 5, Color.red）。
2. GetComponentの呼び出しがUpdateメソッド内で毎回行われるため、パフォーマンスが低下する可能性がある。
3. 複数の責務が1つのメソッドに集約されている（入力処理、物理操作、グラフィック変更）。

保守性が高いコード

using UnityEngine;

public class Player : MonoBehaviour
{
    [SerializeField] private float jumpForce = 10f;
    [SerializeField] private float colorChangeHeight = 5f;
    [SerializeField] private Color highPositionColor = Color.red;

    private Rigidbody2D rb;
    private SpriteRenderer sr;

    void Awake()
    {
        rb = GetComponent<Rigidbody2D>();
        sr = GetComponent<SpriteRenderer>();
    }

    void Update()
    {
        HandleJump();
        HandleColorChange();
    }

    private void HandleJump()
    {
        if (Input.GetKeyDown(KeyCode.Space))
        {
            rb.AddForce(new Vector2(0, jumpForce), ForceMode2D.Impulse);
        }
    }

    private void HandleColorChange()
    {
        if (transform.position.y > colorChangeHeight)
        {
            sr.color = highPositionColor;
        }
    }
}

改善点

1. 重要な値が[SerializeField]属性を使ってインスペクターから変更可能に。
2. コンポーネントの取得をAwakeメソッドで行い、Updateメソッドでの呼び出し回数を削減。
3. 入力処理とグラフィック変更を分離し、責務を明確化。
4. 変数名やメソッド名が意味を持つようにし、可読性を向上。

これにより、コードがより理解しやすく、将来的な変更や拡張が容易になります。

サンプル

保守性が低いコード

using UnityEngine;

public class Enemy : MonoBehaviour
{
    void Update()
    {
        // 移動
        transform.Translate(Vector3.left * 5 * Time.deltaTime);

        // プレイヤーを追いかける
        GameObject player = GameObject.Find("Player");
        if (player != null)
        {
            Vector3 direction = (player.transform.position - transform.position).normalized;
            transform.Translate(direction * 2 * Time.deltaTime);
        }

        // 画面外に出たら削除
        if (transform.position.x < -10)
        {
            Destroy(gameObject);
        }
    }
}

問題点

1. 固定値が多く、変更が難しい（例：5, 2, -10）。
2. 毎フレームGameObject.Findを呼び出しているため、パフォーマンスが低下する。
3. 複数の責務が1つのメソッドに集約されている（移動、プレイヤー追跡、削除）。

保守性が高いコード

using UnityEngine;

public class Enemy : MonoBehaviour
{
    [SerializeField] private float moveSpeed = 5f;
    [SerializeField] private float chaseSpeed = 2f;
    [SerializeField] private float destroyThreshold = -10f;

    private Transform playerTransform;

    void Start()
    {
        GameObject player = GameObject.FindWithTag("Player");
        if (player != null)
        {
            playerTransform = player.transform;
        }
    }

    void Update()
    {
        Move();
        ChasePlayer();
        CheckOutOfBounds();
    }

    private void Move()
    {
        transform.Translate(Vector3.left * moveSpeed * Time.deltaTime);
    }

    private void ChasePlayer()
    {
        if (playerTransform != null)
        {
            Vector3 direction = (playerTransform.position - transform.position).normalized;
            transform.Translate(direction * chaseSpeed * Time.deltaTime);
        }
    }

    private void CheckOutOfBounds()
    {
        if (transform.position.x < destroyThreshold)
        {
            Destroy(gameObject);
        }
    }
}

改善点

1. 固定値を[SerializeField]属性で外部から変更可能に。
2. StartメソッドでGameObject.FindWithTagを使い、毎フレーム呼び出しを避ける。
3. 移動、プレイヤー追跡、削除の各機能を個別のメソッドに分割し、責務を明確化。
4. playerTransformがnullでないかをチェックして、安全性を確保。

このようにすることで、コードの保守性と拡張性が向上し、将来的な変更が容易になります。

これにより、コードがより理解しやすく、将来的な変更や拡張が容易になります。

サンプル

保守性が低いコード

using UnityEngine;

public class GameManager : MonoBehaviour
{
    void Start()
    {
        // プレイヤーを生成
        GameObject player = Instantiate(Resources.Load("PlayerPrefab")) as GameObject;
        player.transform.position = new Vector3(0, 0, 0);

        // 敵を生成
        for (int i = 0; i < 5; i++)
        {
            GameObject enemy = Instantiate(Resources.Load("EnemyPrefab")) as GameObject;
            enemy.transform.position = new Vector3(i * 2, 0, 0);
        }

        // 背景音楽を再生
        AudioSource audioSource = gameObject.AddComponent<AudioSource>();
        AudioClip backgroundMusic = Resources.Load("BackgroundMusic") as AudioClip;
        audioSource.clip = backgroundMusic;
        audioSource.loop = true;
        audioSource.Play();
    }
}

問題点

1. Resources.Loadでリソースを直接ロードしているため、パフォーマンスが低下する可能性がある。
2. ゲームオブジェクトの生成と配置がハードコーディングされている。
3. 音楽再生の処理が直列に並んでおり、可読性が低い。

保守性が高いコード

using UnityEngine;

public class GameManager : MonoBehaviour
{
    [SerializeField] private GameObject playerPrefab;
    [SerializeField] private GameObject enemyPrefab;
    [SerializeField] private AudioClip backgroundMusic;
    [SerializeField] private int enemyCount = 5;
    [SerializeField] private float enemySpacing = 2f;

    private AudioSource audioSource;

    void Start()
    {
        SpawnPlayer();
        SpawnEnemies();
        PlayBackgroundMusic();
    }

    private void SpawnPlayer()
    {
        if (playerPrefab != null)
        {
            GameObject player = Instantiate(playerPrefab, Vector3.zero, Quaternion.identity);
        }
    }

    private void SpawnEnemies()
    {
        if (enemyPrefab != null)
        {
            for (int i = 0; i < enemyCount; i++)
            {
                Vector3 enemyPosition = new Vector3(i * enemySpacing, 0, 0);
                Instantiate(enemyPrefab, enemyPosition, Quaternion.identity);
            }
        }
    }

    private void PlayBackgroundMusic()
    {
        if (backgroundMusic != null)
        {
            audioSource = gameObject.AddComponent<AudioSource>();
            audioSource.clip = backgroundMusic;
            audioSource.loop = true;
            audioSource.Play();
        }
    }
}

改善点

1. プレハブとオーディオクリップを[SerializeField]属性で外部から設定可能にし、ハードコーディングを排除。
2. プレイヤーと敵の生成ロジックを専用のメソッドに分割し、責務を明確化。
3. 敵の数と間隔を[SerializeField]属性で柔軟に変更可能に。
4. 背景音楽の再生処理を専用のメソッドに分割し、可読性を向上。

これにより、コードがより柔軟で管理しやすくなり、将来的な変更や拡張が容易になります。

サンプル

保守性が低いコード

using UnityEngine;

public class ScoreManager : MonoBehaviour
{
    private int score = 0;

    void OnGUI()
    {
        GUI.Label(new Rect(10, 10, 100, 20), "Score: " + score);
    }

    public void AddScore(int points)
    {
        score += points;
    }

    public void ResetScore()
    {
        score = 0;
    }
}

問題点

1. スコア表示のロジックがOnGUIメソッドに直接書かれている。
2. GUIの位置やサイズがハードコーディングされている。
3. スコア表示方法が古いOnGUIメソッドを使用している。

保守性が高いコード

using UnityEngine;
using UnityEngine.UI;

public class ScoreManager : MonoBehaviour
{
    [SerializeField] private Text scoreText;
    private int score = 0;

    void Start()
    {
        UpdateScoreText();
    }

    public void AddScore(int points)
    {
        score += points;
        UpdateScoreText();
    }

    public void ResetScore()
    {
        score = 0;
        UpdateScoreText();
    }

    private void UpdateScoreText()
    {
        if (scoreText != null)
        {
            scoreText.text = "Score: " + score;
        }
    }
}

改善点

1. Textコンポーネントを[SerializeField]属性で外部から設定可能にし、GUIの位置やサイズを柔軟に変更可能に。
2. スコア表示の更新を専用のUpdateScoreTextメソッドに分離し、責務を明確化。
3. OnGUIメソッドを使用せず、UnityのUIシステムを使用することで、最新の方法に対応。

さらに、イベントシステムを使ってスコアを更新する場合の例も示します

保守性が非常に高いコード（イベントシステムを使用）

using UnityEngine;
using UnityEngine.UI;

public class ScoreManager : MonoBehaviour
{
    [SerializeField] private Text scoreText;
    private int score = 0;

    void OnEnable()
    {
        EventManager.OnScoreChanged += UpdateScore;
    }

    void OnDisable()
    {
        EventManager.OnScoreChanged -= UpdateScore;
    }

    private void UpdateScore(int newScore)
    {
        score = newScore;
        UpdateScoreText();
    }

    private void UpdateScoreText()
    {
        if (scoreText != null)
        {
            scoreText.text = "Score: " + score;
        }
    }
}

public static class EventManager
{
    public static System.Action<int> OnScoreChanged;

    public static void ChangeScore(int newScore)
    {
        if (OnScoreChanged != null)
        {
            OnScoreChanged(newScore);
        }
    }
}

改善点

1. イベントシステムを使用することで、スコアの変更を他のスクリプトから簡単に通知できるようにする。
2. OnEnableとOnDisableメソッドを使用して、イベントのサブスクリプションを管理し、適切に解除する。
3. スコア更新のロジックをイベントハンドラ内に移動し、コードの再利用性と拡張性を向上。

このようにすることで、コードがよりモジュール化され、将来的な変更が容易になります。

サンプル

保守性が低いコード

using UnityEngine;

public class HealthManager : MonoBehaviour
{
    private int health = 100;

    void Update()
    {
        if (health <= 0)
        {
            Debug.Log("Game Over");
            Destroy(gameObject);
        }
    }

    public void TakeDamage(int damage)
    {
        health -= damage;
    }

    public void Heal(int amount)
    {
        health += amount;
    }
}

問題点

1. Updateメソッド内で毎フレームhealthをチェックしており、効率が悪い。
2. Debug.Logでゲームオーバーを通知しているが、ゲーム全体の状態管理には適していない。
3. ゲームオブジェクトの削除を直接行っており、ゲームオーバー時の処理が固定されている。

保守性が高いコード

using UnityEngine;
using UnityEngine.Events;

public class HealthManager : MonoBehaviour
{
    [SerializeField] private int maxHealth = 100;
    private int currentHealth;

    public UnityEvent onDeath;

    void Start()
    {
        currentHealth = maxHealth;
    }

    public void TakeDamage(int damage)
    {
        currentHealth -= damage;
        if (currentHealth <= 0)
        {
            currentHealth = 0;
            Die();
        }
    }

    public void Heal(int amount)
    {
        currentHealth += amount;
        if (currentHealth > maxHealth)
        {
            currentHealth = maxHealth;
        }
    }

    private void Die()
    {
        if (onDeath != null)
        {
            onDeath.Invoke();
        }
    }
}

改善点

1. 最大体力を[SerializeField]で設定し、現在の体力を動的に管理。
2. Updateメソッドを使用せず、ダメージを受けるときにのみ体力をチェック。
3. 死亡時の処理をUnityEventで外部から設定可能にし、柔軟性を向上。
4. ヒールの際に体力が最大値を超えないように制御。

保守性が非常に高いコード（イベントシステムを使用）

using UnityEngine;
using UnityEngine.Events;

public class HealthManager : MonoBehaviour
{
    [SerializeField] private int maxHealth = 100;
    private int currentHealth;

    public UnityEvent onDeath;
    public UnityEvent<int> onHealthChanged;

    void Start()
    {
        currentHealth = maxHealth;
        NotifyHealthChanged();
    }

    public void TakeDamage(int damage)
    {
        currentHealth -= damage;
        if (currentHealth <= 0)
        {
            currentHealth = 0;
            Die();
        }
        NotifyHealthChanged();
    }

    public void Heal(int amount)
    {
        currentHealth += amount;
        if (currentHealth > maxHealth)
        {
            currentHealth = maxHealth;
        }
        NotifyHealthChanged();
    }

    private void Die()
    {
        if (onDeath != null)
        {
            onDeath.Invoke();
        }
        Destroy(gameObject);
    }

    private void NotifyHealthChanged()
    {
        if (onHealthChanged != null)
        {
            onHealthChanged.Invoke(currentHealth);
        }
    }
}

改善点

1. UnityEventを使用して、体力が変更されたときに他のスクリプトに通知。
2. NotifyHealthChangedメソッドを追加し、体力が変更されたときにイベントを発火。
3. 死亡時にUnityEventを使用し、外部からの柔軟な反応を可能に。

このようにすることで、体力管理が他のシステムと連携しやすくなり、コードの保守性と拡張性が大幅に向上します。

サンプル

保守性が低いコード

using UnityEngine;

public class EnemySpawner : MonoBehaviour
{
    public GameObject enemyPrefab;
    public float spawnInterval = 2f;
    private float nextSpawnTime = 0f;

    void Update()
    {
        if (Time.time > nextSpawnTime)
        {
            Instantiate(enemyPrefab, new Vector3(Random.Range(-10, 10), 0, 0), Quaternion.identity);
            nextSpawnTime = Time.time + spawnInterval;
        }
    }
}

問題点

1. Updateメソッド内で直接敵をスポーンしている。
2. スポーン位置がハードコーディングされており、ランダムな範囲が固定されている。
3. スポーン間隔がハードコーディングされている。

保守性が高いコード

using UnityEngine;

public class EnemySpawner : MonoBehaviour
{
    [SerializeField] private GameObject enemyPrefab;
    [SerializeField] private float spawnInterval = 2f;
    [SerializeField] private float spawnRangeX = 10f;

    private float nextSpawnTime = 0f;

    void Update()
    {
        if (Time.time > nextSpawnTime)
        {
            SpawnEnemy();
            nextSpawnTime = Time.time + spawnInterval;
        }
    }

    private void SpawnEnemy()
    {
        Vector3 spawnPosition = new Vector3(Random.Range(-spawnRangeX, spawnRangeX), 0, 0);
        Instantiate(enemyPrefab, spawnPosition, Quaternion.identity);
    }
}

改善点

1. spawnIntervalとspawnRangeXを[SerializeField]属性で設定し、外部から変更可能に。
2. 敵のスポーンロジックをSpawnEnemyメソッドに分離し、可読性と再利用性を向上。
3. ハードコーディングされた値を排除し、柔軟に設定可能に。

保守性が非常に高いコード（コルーチンを使用）

using UnityEngine;
using System.Collections;

public class EnemySpawner : MonoBehaviour
{
    [SerializeField] private GameObject enemyPrefab;
    [SerializeField] private float spawnInterval = 2f;
    [SerializeField] private float spawnRangeX = 10f;

    void Start()
    {
        StartCoroutine(SpawnEnemies());
    }

    private IEnumerator SpawnEnemies()
    {
        while (true)
        {
            SpawnEnemy();
            yield return new WaitForSeconds(spawnInterval);
        }
    }

    private void SpawnEnemy()
    {
        Vector3 spawnPosition = new Vector3(Random.Range(-spawnRangeX, spawnRangeX), 0, 0);
        Instantiate(enemyPrefab, spawnPosition, Quaternion.identity);
    }
}

改善点

1. Startメソッドでコルーチンを開始し、スッキリとしたタイマー管理を実現。
2. スポーン間隔をWaitForSecondsで管理し、Updateメソッドの負担を軽減。
3. コルーチンを使用することで、スクリプトの可読性とメンテナンス性を向上。

これにより、コードの保守性が向上し、将来的な変更や拡張が容易になります。また、Updateメソッドを使わないことで、コードの効率性も向上します。

サンプル

保守性が低いコード

using UnityEngine;

public class PlayerController : MonoBehaviour
{
    [SerializeField] private float moveSpeed = 5f;

    void Update()
    {
        HandleMovement();
    }

    private void HandleMovement()
    {
        float moveX = 0f;
        float moveZ = 0f;

        if (Input.GetKey(KeyCode.W))
        {
            moveZ = 1f;
        }
        if (Input.GetKey(KeyCode.S))
        {
            moveZ = -1f;
        }
        if (Input.GetKey(KeyCode.A))
        {
            moveX = -1f;
        }
        if (Input.GetKey(KeyCode.D))
        {
            moveX = 1f;
        }

        Vector3 move = new Vector3(moveX, 0, moveZ).normalized;
        transform.Translate(move * moveSpeed * Time.deltaTime, Space.World);
    }
}

問題点

1. 入力処理がUpdateメソッドにハードコーディングされている。
2. 移動速度がハードコーディングされている。
3. 入力処理と移動ロジックが混在している。

保守性が高いコード

using UnityEngine;

public class PlayerController : MonoBehaviour
{
    [SerializeField] private float moveSpeed = 5f;

    void Update()
    {
        HandleMovement();
    }

    private void HandleMovement()
    {
        float moveX = 0f;
        float moveZ = 0f;

        if (Input.GetKey(KeyCode.W))
        {
            moveZ = 1f;
        }
        if (Input.GetKey(KeyCode.S))
        {
            moveZ = -1f;
        }
        if (Input.GetKey(KeyCode.A))
        {
            moveX = -1f;
        }
        if (Input.GetKey(KeyCode.D))
        {
            moveX = 1f;
        }

        Vector3 move = new Vector3(moveX, 0, moveZ).normalized;
        transform.Translate(move * moveSpeed * Time.deltaTime, Space.World);
    }
}

改善点

1. 移動速度を[SerializeField]属性で設定し、外部から変更可能に。
2. 入力処理と移動ロジックをHandleMovementメソッドに分離し、責務を明確化。
3. 入力の合計ベクトルを正規化して、斜め移動時の速度を一定に。

保守性が非常に高いコード（Unityの新しい入力システムを使用）

using UnityEngine;
using UnityEngine.InputSystem;

public class PlayerController : MonoBehaviour
{
    [SerializeField] private float moveSpeed = 5f;
    private Vector2 moveInput;

    void OnEnable()
    {
        var playerInput = new PlayerInput();
        playerInput.Player.Enable();
        playerInput.Player.Move.performed += OnMove;
        playerInput.Player.Move.canceled += OnMove;
    }

    private void OnMove(InputAction.CallbackContext context)
    {
        moveInput = context.ReadValue<Vector2>();
    }

    void Update()
    {
        HandleMovement();
    }

    private void HandleMovement()
    {
        Vector3 move = new Vector3(moveInput.x, 0, moveInput.y).normalized;
        transform.Translate(move * moveSpeed * Time.deltaTime, Space.World);
    }
}

改善点

1. 新しい入力システムを使用して、入力処理をよりモダンで柔軟な方法に移行。
2. OnEnableメソッドで入力アクションを設定し、入力イベントをハンドル。
3. 入力値をOnMoveメソッドで更新し、Updateメソッドで移動処理を実行。
4. 入力システムのアクションマッピングを使用して、入力設定を容易に変更可能に。

このようにすることで、コードの保守性と拡張性が大幅に向上し、将来的な変更や新しい入力デバイスの対応が容易になります。

サンプル

保守性が低いコード

using UnityEngine;

public class DoorController : MonoBehaviour
{
    private bool isOpen = false;

    void Update()
    {
        if (Input.GetKeyDown(KeyCode.E))
        {
            if (isOpen)
            {
                transform.Rotate(0, -90, 0);
                isOpen = false;
            }
            else
            {
                transform.Rotate(0, 90, 0);
                isOpen = true;
            }
        }
    }
}

問題点

1. 入力処理とドアの動作ロジックがUpdateメソッドに混在している。
2. ドアの回転角度がハードコーディングされている。
3. ドアの状態管理が直接変更されており、柔軟性が低い。

保守性が高いコード

using UnityEngine;

public class DoorController : MonoBehaviour
{
    [SerializeField] private float openAngle = 90f;
    [SerializeField] private float closeAngle = 0f;

    private bool isOpen = false;

    void Update()
    {
        if (Input.GetKeyDown(KeyCode.E))
        {
            ToggleDoor();
        }
    }

    private void ToggleDoor()
    {
        if (isOpen)
        {
            CloseDoor();
        }
        else
        {
            OpenDoor();
        }
    }

    private void OpenDoor()
    {
        transform.rotation = Quaternion.Euler(0, openAngle, 0);
        isOpen = true;
    }

    private void CloseDoor()
    {
        transform.rotation = Quaternion.Euler(0, closeAngle, 0);
        isOpen = false;
    }
}

改善点

1. ドアの開閉角度を[SerializeField]属性で設定し、外部から変更可能に。
2. ドアの開閉ロジックをToggleDoorメソッドに分離し、可読性を向上。
3. 開く、閉じる動作をそれぞれOpenDoor、CloseDoorメソッドに分離し、責務を明確化。

保守性が非常に高いコード（アニメーションを使用）

using UnityEngine;

public class DoorController : MonoBehaviour
{
    [SerializeField] private Animator doorAnimator;
    private bool isOpen = false;

    void Update()
    {
        if (Input.GetKeyDown(KeyCode.E))
        {
            ToggleDoor();
        }
    }

    private void ToggleDoor()
    {
        if (isOpen)
        {
            CloseDoor();
        }
        else
        {
            OpenDoor();
        }
    }

    private void OpenDoor()
    {
        doorAnimator.SetTrigger("Open");
        isOpen = true;
    }

    private void CloseDoor()
    {
        doorAnimator.SetTrigger("Close");
        isOpen = false;
    }
}

改善点

1. アニメーターを使用して、ドアの開閉を管理。
2. 開閉動作をアニメーションで制御し、見た目を向上。
3. アニメーションイベントを使用して、柔軟な開閉動作を実現。

このようにすることで、コードの保守性と拡張性が大幅に向上し、アニメーションを使用することで見た目も向上します。将来的な変更や他の動作追加も容易になります。

サンプル

保守性が低いコード

using UnityEngine;

public class LightController : MonoBehaviour
{
    void Update()
    {
        if (Input.GetKeyDown(KeyCode.L))
        {
            GetComponent<Light>().enabled = !GetComponent<Light>().enabled;
        }
    }
}

問題点

1. Updateメソッドで毎フレームGetComponentを呼び出しており、パフォーマンスが低い。
2. Lightコンポーネントの取得がハードコーディングされている。
3. 入力処理とライトのトグルロジックが混在している。

保守性が高いコード

using UnityEngine;

public class LightController : MonoBehaviour
{
    [SerializeField] private Light lightComponent;

    void Start()
    {
        if (lightComponent == null)
        {
            lightComponent = GetComponent<Light>();
        }
    }

    void Update()
    {
        if (Input.GetKeyDown(KeyCode.L))
        {
            ToggleLight();
        }
    }

    private void ToggleLight()
    {
        lightComponent.enabled = !lightComponent.enabled;
    }
}

改善点

1. Lightコンポーネントを[SerializeField]属性で外部から設定可能にし、必要に応じてStartメソッドで取得。
2. ToggleLightメソッドを作成し、入力処理とライトのトグルロジックを分離。
3. UpdateメソッドでのGetComponent呼び出しを削減し、パフォーマンスを向上。

保守性が非常に高いコード（イベントシステムを使用）

using UnityEngine;
using UnityEngine.Events;

public class LightController : MonoBehaviour
{
    [SerializeField] private Light lightComponent;
    public UnityEvent onLightToggle;

    void Start()
    {
        if (lightComponent == null)
        {
            lightComponent = GetComponent<Light>();
        }
    }

    void Update()
    {
        if (Input.GetKeyDown(KeyCode.L))
        {
            ToggleLight();
        }
    }

    private void ToggleLight()
    {
        lightComponent.enabled = !lightComponent.enabled;
        onLightToggle?.Invoke();
    }
}

改善点

1. UnityEventを使用して、ライトのトグル時に他のアクションをトリガー可能に。
2. onLightToggleイベントを設定し、外部から追加のロジックを設定可能に。
3. 入力処理、ライトのトグルロジック、イベントトリガーを分離し、責務を明確化。

このようにすることで、コードの保守性と拡張性が大幅に向上し、他のスクリプトやコンポーネントと柔軟に連携できるようになります。また、将来的な変更や追加機能の実装も容易になります。

サンプル

保守性が低いコード

using UnityEngine;

public class Timer : MonoBehaviour
{
    private float startTime;

    void Start()
    {
        startTime = Time.time;
    }

    void Update()
    {
        if (Time.time - startTime > 10)
        {
            Debug.Log("10 seconds have passed!");
            startTime = Time.time; // Reset the timer
        }
    }
}

問題点

1. タイマーのロジックがUpdateメソッドに埋め込まれており、再利用性が低い。
2. タイマーの時間がハードコーディングされている。
3. タイマーのリセットロジックが直接書かれており、柔軟性が低い。

保守性が高いコード

using UnityEngine;
using UnityEngine.Events;

public class Timer : MonoBehaviour
{
    [SerializeField] private float interval = 10f;
    private float nextTime;

    public UnityEvent onTimerComplete;

    void Start()
    {
        ResetTimer();
    }

    void Update()
    {
        if (Time.time > nextTime)
        {
            onTimerComplete?.Invoke();
            ResetTimer();
        }
    }

    private void ResetTimer()
    {
        nextTime = Time.time + interval;
    }
}

改善点

1. タイマーのインターバルを[SerializeField]属性で設定し、外部から変更可能に。
2. UnityEventを使用して、タイマー完了時に他のアクションをトリガー可能に。
3. タイマーのリセットロジックをResetTimerメソッドに分離し、再利用性を向上。

保守性が非常に高いコード（コルーチンを使用）

using UnityEngine;
using UnityEngine.Events;

public class Timer : MonoBehaviour
{
    [SerializeField] private float interval = 10f;
    public UnityEvent onTimerComplete;

    private Coroutine timerCoroutine;

    void Start()
    {
        StartTimer();
    }

    private void StartTimer()
    {
        if (timerCoroutine != null)
        {
            StopCoroutine(timerCoroutine);
        }
        timerCoroutine = StartCoroutine(TimerCoroutine());
    }

    private IEnumerator TimerCoroutine()
    {
        yield return new WaitForSeconds(interval);
        onTimerComplete?.Invoke();
        StartTimer(); // Restart the timer
    }
}

改善点

1. コルーチンを使用して、タイマー処理を簡潔かつ効率的に実装。
2. UnityEventを使用して、タイマー完了時に他のアクションをトリガー可能に。
3. タイマーの開始とリセットロジックを明確に分離し、コードの再利用性と柔軟性を向上。

このようにすることで、コードの保守性と拡張性が大幅に向上し、タイマー処理が他のシステムと柔軟に連携できるようになります。また、将来的な変更や追加機能の実装も容易になります。

サンプル

保守性が低いコード

using UnityEngine;

public class EnemyHealth : MonoBehaviour
{
    private int health = 100;

    void Update()
    {
        if (health <= 0)
        {
            Debug.Log("Enemy defeated");
            Destroy(gameObject);
        }
    }

    public void TakeDamage(int damage)
    {
        health -= damage;
    }
}

問題点

1. Updateメソッドで毎フレームhealthをチェックしており、効率が悪い。
2. 敵の体力値がハードコーディングされている。
3. Debug.Logを使用しているため、リリース版で不要なログ出力が発生する可能性がある。

保守性が高いコード

using UnityEngine;
using UnityEngine.Events;

public class EnemyHealth : MonoBehaviour
{
    [SerializeField] private int maxHealth = 100;
    private int currentHealth;

    public UnityEvent onDeath;

    void Start()
    {
        currentHealth = maxHealth;
    }

    public void TakeDamage(int damage)
    {
        currentHealth -= damage;
        if (currentHealth <= 0)
        {
            Die();
        }
    }

    private void Die()
    {
        onDeath?.Invoke();
        Destroy(gameObject);
    }
}

改善点

1. 最大体力値を[SerializeField]属性で設定し、外部から変更可能に。
2. TakeDamageメソッドで体力をチェックし、必要な時だけDieメソッドを呼び出す。
3. UnityEventを使用して、死亡時のイベントを外部から設定可能にし、柔軟性を向上。

保守性が非常に高いコード（イベントシステムとインターフェースを使用）

using UnityEngine;
using UnityEngine.Events;

public interface IDamageable
{
    void TakeDamage(int damage);
}

public class EnemyHealth : MonoBehaviour, IDamageable
{
    [SerializeField] private int maxHealth = 100;
    private int currentHealth;

    public UnityEvent onDeath;
    public UnityEvent<int> onHealthChanged;

    void Start()
    {
        currentHealth = maxHealth;
        NotifyHealthChanged();
    }

    public void TakeDamage(int damage)
    {
        currentHealth -= damage;
        if (currentHealth <= 0)
        {
            currentHealth = 0;
            Die();
        }
        NotifyHealthChanged();
    }

    private void Die()
    {
        onDeath?.Invoke();
        Destroy(gameObject);
    }

    private void NotifyHealthChanged()
    {
        onHealthChanged?.Invoke(currentHealth);
    }
}

改善点

1. IDamageableインターフェースを使用して、ダメージ処理を共通化し、他のクラスでも使いやすくする。
2. UnityEventを使用して、体力が変更されたときに他のシステムに通知。
3. onHealthChangedイベントを使用して、体力変更の処理を外部から柔軟に設定可能に。

このようにすることで、コードの保守性と拡張性が大幅に向上し、体力管理が他のシステムと柔軟に連携できるようになります。また、インターフェースを使用することで、異なるクラス間での一貫性が保たれ、再利用性が向上します。

サンプル

保守性が低いコード

using UnityEngine;

public class ItemCollector : MonoBehaviour
{
    private int score = 0;

    void OnTriggerEnter(Collider other)
    {
        if (other.gameObject.tag == "Item")
        {
            score += 10;
            Destroy(other.gameObject);
            Debug.Log("Item collected! Score: " + score);
        }
    }
}

問題点

1. スコアの増加量がハードコーディングされている。
2. OnTriggerEnterメソッド内でスコア管理とアイテムの削除を行っており、責務が曖昧。
3. Debug.Logでスコアを表示しており、リリース版で不要なログ出力が発生する可能性がある。

保守性が高いコード

using UnityEngine;
using UnityEngine.Events;

public class ItemCollector : MonoBehaviour
{
    [SerializeField] private int scoreIncrement = 10;
    private int score = 0;

    public UnityEvent<int> onScoreChanged;

    void OnTriggerEnter(Collider other)
    {
        if (other.CompareTag("Item"))
        {
            CollectItem(other.gameObject);
        }
    }

    private void CollectItem(GameObject item)
    {
        score += scoreIncrement;
        onScoreChanged?.Invoke(score);
        Destroy(item);
    }
}

改善点

1. スコアの増加量を[SerializeField]属性で設定し、外部から変更可能に。
2. スコアの変更をUnityEventで通知し、他のシステムと連携可能に。
3. アイテムの収集ロジックをCollectItemメソッドに分離し、責務を明確化。

保守性が非常に高いコード（イベントシステムとスコア管理クラスを使用）

using UnityEngine;
using UnityEngine.Events;

public class ItemCollector : MonoBehaviour
{
    [SerializeField] private int scoreIncrement = 10;
    public UnityEvent<int> onScoreChanged;

    private ScoreManager scoreManager;

    void Start()
    {
        scoreManager = FindObjectOfType<ScoreManager>();
    }

    void OnTriggerEnter(Collider other)
    {
        if (other.CompareTag("Item"))
        {
            CollectItem(other.gameObject);
        }
    }

    private void CollectItem(GameObject item)
    {
        scoreManager.AddScore(scoreIncrement);
        onScoreChanged?.Invoke(scoreManager.CurrentScore);
        Destroy(item);
    }
}

public class ScoreManager : MonoBehaviour
{
    private int currentScore = 0;

    public int CurrentScore => currentScore;

    public void AddScore(int amount)
    {
        currentScore += amount;
    }

    public void ResetScore()
    {
        currentScore = 0;
    }
}

改善点

1. スコア管理を専用のScoreManagerクラスに分離し、スコア処理の責務を明確化。
2. ScoreManagerを使用して、スコアの追加とリセットを管理。
3. UnityEventを使用して、スコア変更時に他のシステムに通知。
4. ItemCollectorクラスからスコア管理のロジックを分離し、責務を分担。

このようにすることで、コードの保守性と拡張性が大幅に向上し、スコア管理が他のシステムと柔軟に連携できるようになります。また、専用のスコア管理クラスを使用することで、スコア処理が統一され、再利用性が向上します。

サンプル

保守性が低いコード

using UnityEngine;

public class PlayerMovement : MonoBehaviour
{
    void Update()
    {
        if (Input.GetKey(KeyCode.LeftArrow))
        {
            transform.Translate(Vector3.left * 5 * Time.deltaTime);
        }
        if (Input.GetKey(KeyCode.RightArrow))
        {
            transform.Translate(Vector3.right * 5 * Time.deltaTime);
        }
        if (Input.GetKey(KeyCode.UpArrow))
        {
            transform.Translate(Vector3.forward * 5 * Time.deltaTime);
        }
        if (Input.GetKey(KeyCode.DownArrow))
        {
            transform.Translate(Vector3.back * 5 * Time.deltaTime);
        }
    }
}

問題点

1. 入力処理がUpdateメソッドにハードコーディングされている。
2. 移動速度がハードコーディングされている。
3. 入力処理と移動ロジックが混在している。

保守性が高いコード

using UnityEngine;

public class PlayerMovement : MonoBehaviour
{
    [SerializeField] private float moveSpeed = 5f;

    void Update()
    {
        HandleMovement();
    }

    private void HandleMovement()
    {
        float moveX = Input.GetAxis("Horizontal") * moveSpeed * Time.deltaTime;
        float moveZ = Input.GetAxis("Vertical") * moveSpeed * Time.deltaTime;

        transform.Translate(new Vector3(moveX, 0, moveZ));
    }
}

改善点

1. 移動速度を[SerializeField]属性で設定し、外部から変更可能に。
2. Input.GetAxisを使用して、よりスムーズで直感的な入力処理を実現。
3. 入力処理と移動ロジックをHandleMovementメソッドに分離し、可読性と再利用性を向上。

保守性が非常に高いコード（新しい入力システムを使用）

using UnityEngine;
using UnityEngine.InputSystem;

public class PlayerMovement : MonoBehaviour
{
    [SerializeField] private float moveSpeed = 5f;
    private Vector2 moveInput;

    void OnEnable()
    {
        var playerInput = new PlayerInput();
        playerInput.Player.Enable();
        playerInput.Player.Move.performed += OnMove;
        playerInput.Player.Move.canceled += OnMove;
    }

    private void OnMove(InputAction.CallbackContext context)
    {
        moveInput = context.ReadValue<Vector2>();
    }

    void Update()
    {
        HandleMovement();
    }

    private void HandleMovement()
    {
        Vector3 move = new Vector3(moveInput.x, 0, moveInput.y) * moveSpeed * Time.deltaTime;
        transform.Translate(move, Space.World);
    }
}

改善点

1. 新しい入力システムを使用して、入力処理をモダンで柔軟な方法に移行。
2. OnEnableメソッドで入力アクションを設定し、入力イベントをハンドル。
3. OnMoveメソッドで入力値を更新し、Updateメソッドで移動処理を実行。
4. 入力システムのアクションマッピングを使用して、入力設定を容易に変更可能に。

このようにすることで、コードの保守性と拡張性が大幅に向上し、将来的な変更や新しい入力デバイスの対応が容易になります。また、新しい入力システムを使用することで、入力処理がより柔軟で直感的になります。

サンプル

保守性が低いコード

using UnityEngine;

public class PowerUp : MonoBehaviour
{
    void OnTriggerEnter(Collider other)
    {
        if (other.CompareTag("Player"))
        {
            other.GetComponent<Player>().powerLevel += 1;
            Destroy(gameObject);
        }
    }
}

問題点

1. Playerクラスに直接アクセスしてプロパティを変更している。
2. powerLevelの変更がハードコーディングされている。
3. パワーアップのロジックとオブジェクトの破壊が一緒に書かれている。

保守性が高いコード

using UnityEngine;

public class PowerUp : MonoBehaviour
{
    [SerializeField] private int powerIncreaseAmount = 1;

    void OnTriggerEnter(Collider other)
    {
        if (other.CompareTag("Player"))
        {
            Player player = other.GetComponent<Player>();
            if (player != null)
            {
                player.IncreasePower(powerIncreaseAmount);
                Destroy(gameObject);
            }
        }
    }
}

改善点

1. パワーアップの増加量を[SerializeField]属性で設定し、外部から変更可能に。
2. Playerクラスのメソッドを呼び出して、パワーを増加させる。
3. パワーアップのロジックをメソッドに分離し、責務を明確化。

保守性が非常に高いコード（イベントシステムを使用）

using UnityEngine;
using UnityEngine.Events;

public class PowerUp : MonoBehaviour
{
    [SerializeField] private int powerIncreaseAmount = 1;
    public UnityEvent<int> onPowerUpCollected;

    void OnTriggerEnter(Collider other)
    {
        if (other.CompareTag("Player"))
        {
            onPowerUpCollected?.Invoke(powerIncreaseAmount);
            Destroy(gameObject);
        }
    }
}

public class Player : MonoBehaviour
{
    public int powerLevel = 0;

    void OnEnable()
    {
        FindObjectOfType<PowerUp>().onPowerUpCollected.AddListener(IncreasePower);
    }

    public void IncreasePower(int amount)
    {
        powerLevel += amount;
    }
}

改善点

1. UnityEventを使用して、パワーアップ収集時に他のアクションをトリガー可能に。
2. パワーアップの増加量をイベントで通知し、柔軟に他のクラスと連携。
3. Playerクラスでイベントリスナーを設定し、パワーの増加を管理。

保守性が非常に高いコード（インターフェースを使用）

using UnityEngine;
using UnityEngine.Events;

public interface ICollectible
{
    void Collect();
}

public class PowerUp : MonoBehaviour, ICollectible
{
    [SerializeField] private int powerIncreaseAmount = 1;
    public UnityEvent<int> onPowerUpCollected;

    public void Collect()
    {
        onPowerUpCollected?.Invoke(powerIncreaseAmount);
        Destroy(gameObject);
    }

    void OnTriggerEnter(Collider other)
    {
        if (other.CompareTag("Player"))
        {
            Collect();
        }
    }
}

public class Player : MonoBehaviour
{
    public int powerLevel = 0;

    void OnEnable()
    {
        var powerUp = FindObjectOfType<PowerUp>();
        if (powerUp != null)
        {
            powerUp.onPowerUpCollected.AddListener(IncreasePower);
        }
    }

    public void IncreasePower(int amount)
    {
        powerLevel += amount;
    }
}

改善点

1. ICollectibleインターフェースを使用して、収集可能なオブジェクトを共通化。
2. UnityEventを使用して、パワーアップ収集時に他のアクションをトリガー可能に。
3. インターフェースを使用することで、他の収集可能なアイテムとの一貫性を保ち、再利用性を向上。

このようにすることで、コードの保守性と拡張性が大幅に向上し、収集可能なオブジェクトが他のシステムと柔軟に連携できるようになります。また、インターフェースを使用することで、異なるクラス間での一貫性が保たれ、再利用性が向上します。

サンプル

保守性が低いコード

using UnityEngine;

public class GameManager : MonoBehaviour
{
    void Update()
    {
        if (Input.GetKeyDown(KeyCode.P))
        {
            Time.timeScale = 0;
            Debug.Log("Game Paused");
        }

        if (Input.GetKeyDown(KeyCode.R))
        {
            Time.timeScale = 1;
            Debug.Log("Game Resumed");
        }
    }
}

問題点

1. Updateメソッド内で毎フレーム入力をチェックしている。
2. ゲームのポーズと再開のロジックがハードコーディングされている。
3. デバッグ用のDebug.Logがリリース版で不要になる。

保守性が高いコード

using UnityEngine;

public class GameManager : MonoBehaviour
{
    void Update()
    {
        if (Input.GetKeyDown(KeyCode.P))
        {
            PauseGame();
        }
        if (Input.GetKeyDown(KeyCode.R))
        {
            ResumeGame();
        }
    }

    private void PauseGame()
    {
        Time.timeScale = 0;
        Debug.Log("Game Paused");
    }

    private void ResumeGame()
    {
        Time.timeScale = 1;
        Debug.Log("Game Resumed");
    }
}

改善点

1. ゲームのポーズと再開のロジックをメソッドに分離し、責務を明確化。
2. Debug.Logを用いて状態をデバッグしやすくする。

保守性が非常に高いコード（イベントシステムを使用）

using UnityEngine;
using UnityEngine.Events;

public class GameManager : MonoBehaviour
{
    public UnityEvent onPause;
    public UnityEvent onResume;

    void Update()
    {
        if (Input.GetKeyDown(KeyCode.P))
        {
            PauseGame();
        }
        if (Input.GetKeyDown(KeyCode.R))
        {
            ResumeGame();
        }
    }

    private void PauseGame()
    {
        Time.timeScale = 0;
        onPause?.Invoke();
    }

    private void ResumeGame()
    {
        Time.timeScale = 1;
        onResume?.Invoke();
    }
}

改善点

1. UnityEventを使用して、ポーズと再開のイベントを他のスクリプトと連携可能に。
2. イベントを使用することで、ポーズや再開時に追加の処理を容易に追加可能。

保守性が非常に高いコード（コマンドパターンを使用）

using UnityEngine;
using UnityEngine.Events;

public class GameManager : MonoBehaviour
{
    public UnityEvent onPause;
    public UnityEvent onResume;

    private ICommand pauseCommand;
    private ICommand resumeCommand;

    void Start()
    {
        pauseCommand = new PauseCommand(onPause);
        resumeCommand = new ResumeCommand(onResume);
    }

    void Update()
    {
        if (Input.GetKeyDown(KeyCode.P))
        {
            ExecuteCommand(pauseCommand);
        }
        if (Input.GetKeyDown(KeyCode.R))
        {
            ExecuteCommand(resumeCommand);
        }
    }

    private void ExecuteCommand(ICommand command)
    {
        command.Execute();
    }
}

public interface ICommand
{
    void Execute();
}

public class PauseCommand : ICommand
{
    private UnityEvent onPause;

    public PauseCommand(UnityEvent onPause)
    {
        this.onPause = onPause;
    }

    public void Execute()
    {
        Time.timeScale = 0;
        onPause?.Invoke();
    }
}

public class ResumeCommand : ICommand
{
    private UnityEvent onResume;

    public ResumeCommand(UnityEvent onResume)
    {
        this.onResume = onResume;
    }

    public void Execute()
    {
        Time.timeScale = 1;
        onResume?.Invoke();
    }
}

改善点

1. コマンドパターンを使用して、ポーズと再開のロジックをコマンドオブジェクトに分離。
2. コマンドオブジェクトを使用することで、ポーズと再開の処理を簡単に拡張可能に。
3. UnityEventを使用して、イベントドリブンなアーキテクチャを実現。

このようにすることで、コードの保守性と拡張性が大幅に向上し、ゲームのポーズや再開処理が他のシステムと柔軟に連携できるようになります。また、コマンドパターンを使用することで、将来的な変更や新しい機能の追加が容易になります。