How Could I Simplify My Code and Make It More Efficient?

Unity Engine
,
1

Hello,

I’ve been experimenting with 2D Enemy AI movement, and I got something I’m pretty happy with. However, there are just a lot of checks (whether they are next to something, should jump, etc.) and my code is just very long. I’m worried it might cause problems later on.

I don’t know if simple AI scripts are typically very long, but I was just wondering if there was anything I could simplify down.

Just for background, I used scriptable objects to determine the move sets for the enemies. That way I can make different types of enemies with just one script.

Anyways, here’s the code:

using System.Collections;
using System.Collections.Generic;
using UnityEngine;

public class Enemy : MonoBehaviour
{
    public List<AIMoveEvent> moveEvents;
    string chosenMoveName;

    int AIMoveEvent_Number;

    public Rigidbody2D rb;

    public float health;
    public float speed;
    public bool isGrounded;
    public bool isOnTopOfAnotherEnemy;

    bool AIMoveCoroutine_isRunning;
    RaycastHit2D hitDown, hitDownLeft, hitDownRight;
    RaycastHit2D hitRight;
    RaycastHit2D hitLeft;

    // Start is called before the first frame update
    void Start()
    {
        rb = GetComponent<Rigidbody2D>();
        StartCoroutine("AIMove_Event");

    }

    // Update is called once per frame
    void Update()
    {
        GroundedChecks();



        hitDown = Physics2D.Raycast(new Vector2(transform.position.x, transform.position.y - 0.7f), Vector2.down, 0.3f);
        Debug.DrawRay(new Vector2(transform.position.x, transform.position.y - 0.7f), Vector2.down, Color.green);
       
        hitDownRight = Physics2D.Raycast(new Vector2(transform.position.x + 0.2f, transform.position.y - 0.7f), Vector2.down, 0.3f);
        Debug.DrawRay(new Vector2(transform.position.x + 0.2f, transform.position.y - 0.7f), Vector2.down, Color.green);
       
        hitDownLeft = Physics2D.Raycast(new Vector2(transform.position.x - 0.2f, transform.position.y - 0.7f), Vector2.down, 0.3f);
        Debug.DrawRay(new Vector2(transform.position.x - 0.2f, transform.position.y - 0.7f), Vector2.down, Color.green);

        hitRight = Physics2D.Raycast(new Vector2(transform.position.x, transform.position.y + 0.7f), Vector2.right, 0.5f);
        Debug.DrawRay(new Vector2(transform.position.x + 0.7f, transform.position.y), Vector2.right, Color.red);
     
        hitLeft = Physics2D.Raycast(new Vector2(transform.position.x - 0.7f, transform.position.y), Vector2.left, 0.5f);
        Debug.DrawRay(new Vector2(transform.position.x - 0.7f, transform.position.y), Vector2.left, Color.red);

      
       
       
        AIMoveEvent chosenMove = moveEvents[AIMoveEvent_Number];
        chosenMoveName = chosenMove.name;
        print(chosenMove);
      
        if(chosenMoveName == "MoveLeft")
        {
            MoveLeft();
        }
      
        if(chosenMoveName == "MoveRight")
        {
            MoveRight();
        }
   
        if(health <= 0)
        {
            Destroy(gameObject);
        }
   
    }

    void Jump()
    {
        if (!AIMoveCoroutine_isRunning)
        {
            StartCoroutine("AIMove_Event");
        }

        if (!isOnTopOfAnotherEnemy)
        {
            StopCoroutine("AIMove_Event");
            AIMoveCoroutine_isRunning = false;
            rb.AddForce(Vector2.up * 2.5f, ForceMode2D.Impulse);
        }

    }

    void MoveLeft()
    {
        if (!AIMoveCoroutine_isRunning)
        {
            StartCoroutine("AIMove_Event");
        }

        if (hitLeft.collider != null && isGrounded)
        {
            int choice = Random.Range(1, 3);
            if (choice == 1 && !isOnTopOfAnotherEnemy )
            {
                StopCoroutine("AIMove_Event");
                AIMoveCoroutine_isRunning = false;
                Jump();
            }
            if (choice == 2 || isOnTopOfAnotherEnemy)
            {
                StopCoroutine("AIMove_Event");
                AIMoveCoroutine_isRunning = false;
                chosenMoveName = "MoveRight";
            }
        }

        else if (hitLeft.collider != null && !isGrounded)
        {
            chosenMoveName = "MoveRight";
        }
        else if (hitLeft.collider != null && hitLeft.collider.CompareTag("Enemy"))
        {
            int choice = Random.Range(1, 3);
           
            if (choice == 1)
            {
                StopCoroutine("AIMove_Event");
                AIMoveCoroutine_isRunning = false;
                Jump();
            }
            if (choice == 2)
            {
                StopCoroutine("AIMove_Event");
                AIMoveCoroutine_isRunning = false;
                chosenMoveName = "MoveRight";
            }

        }

        else
        {
            transform.Translate(Vector2.left * speed * Time.deltaTime);
        }


    }

    void MoveRight()
    {
        if (!AIMoveCoroutine_isRunning)
        {
            StartCoroutine("AIMove_Event");
        }

        if (hitRight.collider != null && isGrounded)
        {
            int choice = Random.Range(1, 3);

            if(choice == 1 && !isOnTopOfAnotherEnemy)
            {
                StopCoroutine("AIMove_Event");
                AIMoveCoroutine_isRunning = false;
                Jump();
            }
            if(choice == 2 || isOnTopOfAnotherEnemy)
            {
                StopCoroutine("AIMove_Event");
                AIMoveCoroutine_isRunning = false;
                chosenMoveName = "MoveLeft";
            }
        }
       
        else if(hitRight.collider != null && !isGrounded)
        {
            chosenMoveName = "MoveLeft";
        }
        else if(hitRight.collider != null && hitRight.collider.CompareTag("Enemy"))
        {
           
            int choice = Random.Range(1, 3);

            if (choice == 1)
            {
                StopCoroutine("AIMove_Event");
                AIMoveCoroutine_isRunning = false;
                Jump();
            }
            if (choice == 2)
            {
                StopCoroutine("AIMove_Event");
                AIMoveCoroutine_isRunning = false;
                chosenMoveName = "MoveLeft";
            }
        }
        else
        {
             transform.Translate(Vector2.right * speed * Time.deltaTime);
        }
    }

  
    IEnumerator AIMove_Event()
    {
        yield return new WaitForEndOfFrame();
        AIMoveCoroutine_isRunning = true;
        StopCoroutine("AIMove_Timer");

        AIMoveEvent_Number = Random.Range(0, moveEvents.Capacity);
        print(AIMoveEvent_Number);

        StartCoroutine("AIMove_Timer");

    }
   
    IEnumerator AIMove_Timer()
    {
        yield return new WaitForEndOfFrame();

        int timerValue = Random.Range(1, 5);
        yield return new WaitForSeconds(timerValue);

        StopCoroutine("AIMove_Event");
        AIMoveCoroutine_isRunning = false;
        StartCoroutine("AIMove_Event");

    }

    void GroundedChecks()
    {

        if (hitDown.collider != null)
        {
            isGrounded = true;

            if (!hitDown.collider.gameObject.CompareTag("Enemy"))
            {
                isOnTopOfAnotherEnemy = false;
            }
            if (hitDown.collider.gameObject.CompareTag("Enemy"))
            {
                isOnTopOfAnotherEnemy = true;
            }

        }
        if (hitDownRight.collider != null)
        {
            isGrounded = true;

            if (!hitDownRight.collider.gameObject.CompareTag("Enemy"))
            {
                isOnTopOfAnotherEnemy = false;
            }
            if (hitDownRight.collider.gameObject.CompareTag("Enemy"))
            {
                isOnTopOfAnotherEnemy = true;
            }

        }
        if (hitDownLeft.collider != null)
        {
            isGrounded = true;

            if (!hitDownLeft.collider.gameObject.CompareTag("Enemy"))
            {
                isOnTopOfAnotherEnemy = false;
            }
            if (hitDownLeft.collider.gameObject.CompareTag("Enemy"))
            {
                isOnTopOfAnotherEnemy = true;
            }

        }

        else
        {
            isGrounded = false;
            isOnTopOfAnotherEnemy = false;
        
        }
       
        if (isOnTopOfAnotherEnemy)
        {
        
            int choice = Random.Range(1, 3);

            if (choice == 1)
            {
                chosenMoveName = "MoveRight";

            }
            if (choice == 2)
            {
                chosenMoveName = "MoveLeft";
            }

        }
    }

    private void OnTriggerEnter2D(Collider2D collision)
    {
        if(collision.name == "GroundPoundTrigger")
        {
            health = health - 5;
        }
    }
}

Thanks in advance for any feedback!

2

Whether this code would be problematic is really dependent on how many enemies you expect to have in the scene running this logic. If it’s just a few I doubt there will be any issues. However, here are some notes on things I could see improved:

  • There is no reason to create a new WaitForEndOfFrame intance everytime you wish to yield until the end of frame. Just create one instance in Awake or Start and use for all of those yields.
  • Personally, I would try to encapsulate the movement logic (MoveLeft/MoveRight) in the AIMoveEvent classes themselves. Create a base abstract AIMoveEvent class with an abstract Move method, then create child classes that implement the Move method in whatever direction you want. This will eliminate the chosenMove.name string comparison, as you can just call Move and it will perform the correct type of movement.
  • It would take too much to fully understand the purpose of the of the AIMove_Event and AIMove_Timer methods and the reason for calling StartCoroutine/StopCoroutine so often, but if I had to guess I’d say you could find a more efficient way of achieving whatever you are trying to achieve here. Again though, it probably won’t be an issue unless you have many enemies. Just keep in mind that every time you call StartCoroutine, a bit of garbage is generated.

This certainly isn’t everything that could be “improved.” In general, people will probably advise you to not worry too much about improving working code unless you see it become a problem (via profiling). The reason is, you can end up spending a bunch of time on something that really was never an issue to begin with.

However, personally I think if you are a beginner it doesn’t hurt to gather this type of feedback so that you can write more efficient code from the start. Good luck!

1 Like
3

Some things I noticed.

StartCoroutine("AIMove_Event");

Do not start coroutines using their name. If you rename the function, things will break with no warning.

See example here: Unity - Scripting API: MonoBehaviour.StartCoroutine
And use StartCoroutine(AiMove_Event()).

if(chosenMoveName == "MoveLeft")

Use switch-case. Additionally consider storing functiosn being called within the event itself. See System.Func, System.Action, idea of delegates.

       hitDown = Physics2D.Raycast(new Vector2(transform.position.x, transform.position.y - 0.7f), Vector2.down, 0.3f);
        Debug.DrawRay(new Vector2(transform.position.x, transform.position.y - 0.7f), Vector2.down, Color.green);

This pattern repeats several times, and can be probably turned into a function with parameters.

        if (hitLeft.collider != null && isGrounded)
        {
            int choice = Random.Range(1, 3);
            if (choice == 1 && !isOnTopOfAnotherEnemy )

EVERY if/else branch has “hitCollider != null” in it, meaning it can be moved into outer scope, something like “if (hitLeft.collider == null) return”.

Code for left and right movement is nearly identical, meaning it should be refactored into a single function with parameters.

            if (!hitDown.collider.gameObject.CompareTag("Enemy"))
            {
                isOnTopOfAnotherEnemy = false;
            }
            if (hitDown.collider.gameObject.CompareTag("Enemy"))
            {
                isOnTopOfAnotherEnemy = true;
            }

Replace with:
isOnTopOfAnotherEnemy = hitDown.collider.gameObject.CompareTag("Enemy")

        else
        {
            isGrounded = false;
            isOnTopOfAnotherEnemy = false;
       
        }

You can remove this else completely, just set the values BEFORE if/else branch.

            isGrounded = false;
            isOnTopOfAnotherEnemy = false;
           if (...)

With previous fix you can remove the whole if/else tree.

if(collision.name == "GroundPoundTrigger")

If you forget to name collider correctly, the code will break. You WILL forget to name the collider correctly.

Instead of checking tags or comparing names, you can create component for enemy and world colliders and check for their presence with GetComponent.

2 Likes
4

@neginfinity has a bunch of good tips.

But number zero on the list is always: get it to work first, and then measure your performance with the profiler. Always work on the #1 worst piece of code first, and then when it’s no longer #1, work on the new #1. Every moment you spend trying to make the #16th worst piece of code faster is your time wasted.

1 Like
5

Instead of having two almost identical functions that move left and right you could just have a single function that moves left or right. You could also drop the use of coroutines as they’re not really necessary.

Use AddForce to move your enemy left or right and if the rb.velocity drops then you can assume it has bumped into something and reverse its move direction. This will save you from having to do all those collision checks.

You should be able to squish the whole thing into about 50 lines of code.

6

The simple fix for that is to use the nameof expression:

StartCoroutine(nameof(AIMove_Event));

But I would generally advice against using several coroutines. It just gets complicated to manage which coroutine is running and which isn’t. A simple statemachine can be implemented with switch or if/else constructs.

choice == 1

This is non-expressive code and a real maintenance headache. A month from now will you be able to remember what choice 12 is? Use an enum for that:

choice == AIState.Attacking

DRY principle:

if(hitRight.collider != null && isGrounded)
..
else if(hitRight.collider != null && !isGrounded)
..
else if(hitRight.collider != null && hitRight.collider.CompareTag("Enemy"))
..

Don’t repeat checking for null in every condition! Not only will this run the check unnecessarily often, it also makes the code harder to read.

In addition, avoid explicitly writing opposite / negated conditions. Did you notice that the above if/else if/else construct will NEVER execute the third block? :wink:

Here’s the simplified form of the conditions:

if(isGrounded)
..
else if(!isGrounded)
..
else if (something else)
..

You see it now?

You’ll spot this IMMEDIATELY in the corrected version:

if(isGrounded)
..
else // if we get here, isGrounded is DEFINITELY false
..

That is why you should NEVER ever write code that checks one condition, and then else if’s on the negated condition.

This also tells me that you haven’t tested your code. There’s a seemingly important portion of your code that NEVER executes and you did not notice that. How come?

Best practice programming rule NUMBER ONE: test your code! ALL OF IT! :wink:
(well at least what’s logically important, you needn’t test every error condition)

Lastly, about your IsGrounded checks … well here’s how I would write it:

private void GroundedChecks()
{
    var downColliderValid = hitDown.collider != null;
    var rightColliderValid = hitDownRight.collider != null;
    var leftColliderValid = hitDownLeft.collider != null;
    isGrounded = downColliderValid || rightColliderValid || leftColliderValid;
    isOnTopOfAnotherEnemy = downColliderValid && hitDown.collider.CompareTag("Enemy") ||
                            rightColliderValid && hitDownRight.collider.CompareTag("Enemy") ||
                            leftColliderValid && hitDownLeft.collider.CompareTag("Enemy");
}

Notice that the collider has the CompareTag method too since it is declared in the Component class which both Collider and GameObject inherit from.

1 Like
7

Not 100% sure I got all your logic right but this is a rewrite with some comments :slight_smile:

using System.Collections;
using System.Collections.Generic;
using UnityEngine;

// Added to make my IDE happy.
public class AIMoveEvent
{
    public string name;
}

public class Enemy : MonoBehaviour
{
    // Not a fan of strings, but if you use them then please only define them in one spot.
    // It's just too easy to mess those up
    public const string MoveLeftName = "MoveLeft";
    public const string MoveRightName = "MoveRight";
    public const string EnemyTag = "Enemy";
    public const string GroundPoundTrigger = "GroundPoundTrigger";

    public List<AIMoveEvent> moveEvents;

    // You are mixing naming converntions. Use _ or camel case. Using both is confusing.
    // Also it's a convention among programmes to call this an INDEX, not number (nitpick).
    // Also some of your members start with lower case (health, speed, ...) but this starts upper case.
    int AIMoveEvent_Number;

    // I'd rather store reference to the current move than a copy of the string.
    // Scratch that: How about full normalization and access the elements directly.
    // Another improvement would be that by updating "AIMoveEvent_Number" you automatically
    // "update" the chosenMove. You can NOT not update it.
    AIMoveEvent chosenMove => moveEvents[AIMoveEvent_Number];

    public Rigidbody2D rb;
    public float health;
    public float speed;
    public bool isGrounded;
    public bool isOnTopOfAnotherEnemy;

    // You may want to change this if performance is hit by it (I doubt it).
    public bool isMoving => AIMoveCoroutine != null;

    // Instead of using an extra bool you could store a reference to the coroutine.
    // It has the added benefit that you can easily stop it anytime and you can not
    // forget to update isRunning.
    // bool AIMoveCoroutine_isRunning;
    Coroutine AIMoveCoroutine;
    RaycastHit2D hitDown, hitDownLeft, hitDownRight;
    RaycastHit2D hitRight;
    RaycastHit2D hitLeft;

    void Start()
    {
        rb = GetComponent<Rigidbody2D>();
        startMovement();
    }

    // Update is called once per frame
    void Update()
    {
        GroundedChecks();
        Move();

        if (health <= 0)
        {
            Destroy(gameObject);
        }
    }

    private void Move()
    {
        // DRY Principle ([D]on't [R]epeat [Y]ourself). Make repeating code into a method.
        // That way it's also easier to grasp what is going on (helps with debugging).
        hitDown = checkDirection(startX: 0f, startY: -0.7f, direction: Vector2.down, distance: 0.3f, debug: true, debugColor: Color.green);
        hitDownRight = checkDirection(startX: 0.2f, startY: -0.7f, direction: Vector2.down, distance: 0.3f, debug: true, debugColor: Color.green);
        hitDownLeft = checkDirection(startX: -0.2f, startY: -0.7f, direction: Vector2.down, distance: 0.3f, debug: true, debugColor: Color.green);
        hitRight = checkDirection(startX: 0.7f, startY: 0f, direction: Vector2.right, distance: 0.5f, debug: true, debugColor: Color.red);
        hitLeft = checkDirection(startX: 0.7f, startY: 0f, direction: Vector2.left, distance: 0.5f, debug: true, debugColor: Color.red);

        print(chosenMove.name);

        // Don't use strings. How about an enum or a reference to the move object itself.
        // A switch might be nice here, though not sure.
        if (chosenMove.name == MoveLeftName)
        {
            MoveLeft(); // MoveLeft() can change the chosenMove. That might be unexpected for future you.
                        // Maybe consider making this a return value.
        }

        if (chosenMove.name == MoveRightName)
        {
            MoveRight(); // MoveRight() can also change the choseMove
        }
    }

    private RaycastHit2D checkDirection(float startX, float startY, Vector2 direction, float distance, bool debug, Color debugColor)
    {
#if UNITY_EDITOR
        if (debug)
        {
            Debug.DrawRay(new Vector2(transform.position.x + startX, transform.position.y + startY), direction, debugColor);
        }
#endif

        return Physics2D.Raycast(new Vector2(transform.position.x + startX, transform.position.y + startY), direction, distance);
    }

    void Jump()
    {
        startMovement();

        if (!isOnTopOfAnotherEnemy)
        {
            stopMovement();
            rb.AddForce(Vector2.up * 2.5f, ForceMode2D.Impulse);
        }

    }

    private void startMovement()
    {
        if (isMoving)
            return;

        if (AIMoveCoroutine == null)
        {
            // No string needed. You can use AIMove_Event() here.
            AIMoveCoroutine = StartCoroutine(AIMove_Event());
        }
    }

    private void stopMovement()
    {
        if (!isMoving)
            return;

        if (AIMoveCoroutine != null)
        {
            StopCoroutine(AIMoveCoroutine);
        }
        AIMoveCoroutine = null;
    }

    void MoveLeft()
    {
        Move(getMoveEventIndexByName(MoveLeftName), hitLeft, Vector2.left);
    }

    void MoveRight()
    {
        Move(getMoveEventIndexByName(MoveRightName), hitRight, Vector2.right);
    }

    void Move(int movementIndex, RaycastHit2D hit, Vector2 defaultMovement)
    {
        startMovement();

        if (hit.collider != null)
        {
            if (isGrounded)
            {
                bool choice = getRandomBool();
                if (choice && !isOnTopOfAnotherEnemy)
                {
                    stopMovement();
                    Jump();
                }
                if (!choice || isOnTopOfAnotherEnemy)
                {
                    stopMovement();
                    AIMoveEvent_Number = movementIndex;
                }
            }
            else
            {
                AIMoveEvent_Number = movementIndex;
            }
            // In your origianl code this would never be executed!
            // Not sure where it belongs.
            /*
            else if (hit.collider.CompareTag(EnemyTag))
            {
                bool choice = getRandomBool();
                if (choice)
                {
                    stopMovement();
                    Jump();
                }
                else
                {
                    stopMovement();
                    AIMoveEvent_Number = movementIndex;
                }
            }
            */
        }
    
        // Not sure if this should always be executed.
        // Your if-else constructs in the move methods are definitely faulty.
        transform.Translate(defaultMovement * speed * Time.deltaTime);
    }

    private static bool getRandomBool()
    {
        return Random.Range(0, 2) == 1;
    }

    // This you could replace if needed due to performance but I doubt it will ever be necessary.
    private int getMoveEventIndexByName(string name)
    {
        for (int i = 0; i < moveEvents.Count; i++)
        {
            if (moveEvents[i].name == name)
                return i;
        }

        return -1;
    }

    IEnumerator AIMove_Event()
    {
        yield return new WaitForEndOfFrame();

        // U sure you want to use CAPACITY here not COUNT?!
        // Using CAPACITY might give you and IndexOutOfBoundsException when used.
        // AIMoveEvent_Number = Random.Range(0, moveEvents.Capacity);
        AIMoveEvent_Number = Random.Range(0, moveEvents.Count);
        print(AIMoveEvent_Number);

        yield return new WaitForEndOfFrame();

        int timerValue = Random.Range(1, 5);
        yield return new WaitForSeconds(timerValue);

        // Repeating coroutine. Maybe think about mocing this to Update().
        // Coroutines are a costly way of doing this.
        stopMovement();
        startMovement();
    }

    private void GroundedChecks()
    {
        // By courtesy of CodeSmile
        var downColliderValid = hitDown.collider != null;
        var rightColliderValid = hitDownRight.collider != null;
        var leftColliderValid = hitDownLeft.collider != null;
        isGrounded = downColliderValid || rightColliderValid || leftColliderValid;
        isOnTopOfAnotherEnemy = downColliderValid && hitDown.collider.CompareTag(EnemyTag) ||
                                rightColliderValid && hitDownRight.collider.CompareTag(EnemyTag) ||
                                leftColliderValid && hitDownLeft.collider.CompareTag(EnemyTag);

        if (isOnTopOfAnotherEnemy)
        {
            bool moveRight = getRandomBool();
            if (moveRight)
            {
                AIMoveEvent_Number = getMoveEventIndexByName(MoveRightName);
            }
            else
            {
                AIMoveEvent_Number = getMoveEventIndexByName(MoveLeftName);
            }
        }
    }

    private void OnTriggerEnter2D(Collider2D collision)
    {
        if (collision.CompareTag(GroundPoundTrigger))
        {
            ChangeHealthBy(-5);
        }
    }

    private void ChangeHealthBy(float delta)
    {
        health = Mathf.Max(0, health + delta); // Often you don't want health to become negative.
                                           // Not sure, just something I like to do.
    }
}

One of the recurring things I noticed is that you have some information redundancy in there. Eliminating those will make your code much easier to maintain (see my comments in the code).

Your Move…() if else constructs are definitely faulty (some code is never executed).

Your coroutine nesting was unnecessary (if I interpreted what you want to do correctly).

8 
using UnityEngine;
public class Enemy : MonoBehaviour
{
    Rigidbody2D rb;
    public float health;
    Vector2 moveDirection=Vector2.right;

    void Start()
    {
        rb=GetComponent<Rigidbody2D>();
        Physics2D.queriesStartInColliders=false;
    }

    void FixedUpdate()
    {
        if (rb.velocity.magnitude<1 && Physics2D.CircleCast(transform.position,0.4f,Vector2.down,1.0f) && Physics2D.CircleCast(transform.position,0.4f,moveDirection,1.0f))  // are we grounded and have hit a wall?
        {
            if (Random.Range(0,2)>0)
                rb.AddForce(Vector2.up*20,ForceMode2D.Impulse);  // sometimes jump
            else
                moveDirection=-moveDirection;   // sometimes change direction
        }
        else
            rb.AddForce(moveDirection*30f);    // move left or right

        if (health<=0)
            Destroy(gameObject);
    }
   
    void OnCollisionEnter2D(Collision2D collision)
    {
        if (collision.gameObject.name=="GroundPoundTrigger")
            health=health-5;
    }
}
9

There’s no reason to have both movement and health handling in the same component. Extracting all the health related stuff into its own component would improve readability on the whole.

In general long scripts aren’t necessarily bad, if they

  • hide the complexity of the implementation details and are simple for other scripts to work with in code,
  • have good cohesion (everything inside the classes feel like they belong together; they don’t have multiple unrelated responsibilities),
  • and can be extended without making modifications to the existing code (or probably will not have to be extended at all).

It’s when you have many classes tightly intertwined together in a spaghetti code mess, or a really long and complicated class that needs to be modified often, that bugs tend to become a big problem.