Press "Enter" to skip to content

Lesson 12.6: Refactoring after adding Actions

We’ve made many changes recently. Let’s take a moment to do some refactoring.

Step 1: Create Engine\Actions\BaseAction.cs

All our action classes will share some similar functions. So, let’s put them in a base class.

We never need a BaseAction object. We’ll only instantiate children of BaseAction. So, we’re making this an “abstract” class, which cannot be instantiated on its own – only through its child classes.

Move the OnActionPerformed event and ReportResult function into BaseAction. Notice that the event is still public, because other objects will subscribe to this event (the LivingEntity class).

However, the ReportResult function and _itemInUse variable are protected. This is so BaseAction’s child classes can use them.

I’ve also created a protected BaseAction constructor that accepts a GameItem parameter “itemInUse” and saves it to the protected “_itemInUse” variable.

BaseAction.cs
using System;
using Engine.Models;

namespace Engine.Actions
{
    public abstract class BaseAction
    {
        protected readonly GameItem _itemInUse;

        public event EventHandler<string> OnActionPerformed;

        protected BaseAction(GameItem itemInUse)
        {
            _itemInUse = itemInUse;
        }

        protected void ReportResult(string result)
        {
            OnActionPerformed?.Invoke(this, result);
        }
    }
}

Step 2: Modify Engine\Actions\AttackWithWeapon.cs and Heal.cs

Now we can remove the code in the new base class from its children.

Remove the OnActionPerformed event, the ReportResult function, the _weapon variable from AttackWithWeapon, and the

I’ve also renamed the “weapon” parameter and variable from AttackWithWeapon, and the “item” parameter and variable from Heal. We’ll use the _itemInUse variable from BaseAction.

We’re combining an abstract base class with an interface here.

If we implemented all the interface requirements in the base class, we could change BaseAction to “public abstract class BaseAction : IAction”, and remove the ” : IAction” from the child classes. The child classes would still be IAction objects, because their parent class is an IAction object.

But, in this situation, we cannot put the Execute function (which is required in the interface) in the base class. Each Action child class has unique code in their Execute function. So, we need to have ” : IAction” in each child class.

AttackWithWeapon.cs
using System;
using Engine.Models;

namespace Engine.Actions
{
    public class AttackWithWeapon : BaseAction, IAction
    {
        private readonly int _maximumDamage;
        private readonly int _minimumDamage;

        public AttackWithWeapon(GameItem itemInUse, int minimumDamage, int maximumDamage)
            : base(itemInUse)
        {
            if(itemInUse.Category != GameItem.ItemCategory.Weapon)
            {
                throw new ArgumentException($"{itemInUse.Name} is not a weapon");
            }

            if(_minimumDamage < 0)
            {
                throw new ArgumentException("minimumDamage must be 0 or larger");
            }

            if(_maximumDamage < _minimumDamage)
            {
                throw new ArgumentException("maximumDamage must be >= minimumDamage");
            }

            _minimumDamage = minimumDamage;
            _maximumDamage = maximumDamage;
        }

        public void Execute(LivingEntity actor, LivingEntity target)
        {
            int damage = RandomNumberGenerator.NumberBetween(_minimumDamage, _maximumDamage);

            string actorName = (actor is Player) ? "You" : $"The {actor.Name.ToLower()}";
            string targetName = (target is Player) ? "you" : $"the {target.Name.ToLower()}";

            if(damage == 0)
            {
                ReportResult($"{actorName} missed {targetName}.");
            }
            else
            {
                ReportResult($"{actorName} hit {targetName} for {damage} point{(damage > 1 ? "s" : "")}.");

                target.TakeDamage(damage);
            }
        }
    }
}
Heal.cs
using System;
using Engine.Models;

namespace Engine.Actions
{
    public class Heal : BaseAction, IAction
    {
        private readonly int _hitPointsToHeal;

        public Heal(GameItem itemInUse, int hitPointsToHeal)
            : base(itemInUse)
        {
            if (itemInUse.Category != GameItem.ItemCategory.Consumable)
            {
                throw new ArgumentException($"{itemInUse.Name} is not consumable");
            }

            _hitPointsToHeal = hitPointsToHeal;
        }

        public void Execute(LivingEntity actor, LivingEntity target)
        {
            string actorName = (actor is Player) ? "You" : $"The {actor.Name.ToLower()}";
            string targetName = (target is Player) ? "yourself" : $"the {target.Name.ToLower()}";

            ReportResult($"{actorName} heal {targetName} for {_hitPointsToHeal} point{(_hitPointsToHeal > 1 ? "s" : "")}.");
            target.Heal(_hitPointsToHeal);
        }
    }
}

Step 3: Run the unit tests and play the game, to ensure the refactoring changes didn’t break anything.

Leave a Reply

Your email address will not be published. Required fields are marked *