Refactoring BaseAction to contain an abstract PerformAction() method

[Timothy_McCune]

Since we can assume that all Actions will have some sort of behavior, wouldn’t it make sense to add the following to our BaseAction class:

private void Update()
{
    if (!isActive) { return; }
    PerformAction();
}

// Must be implemented by derived classes:
protected abstract void PerformAction();

Then within our MoveAction and SpinAction, move all logic related to the action into the protected override void PerformAction() methods?

Just my 2.222223 cents.

[bixarrio]

While I don’t mind this, it is somewhat pointless. A bit like

public void DoSomething()
{
    PerformSomething();
}

private abstract void PerformSomething();

// or worst case
public int Add(int a, int b)
{
    return PerformAddition(a, b);
}

private abstract int PerformAddition(int a, int b);

All actions already have an Update method that is just going to call PerformAction(). Granted, it removes the need to remember to do the isActive check, but is that enough?

In addition, my GrenadeAction does nothing in Update, so now I am forced to implement a method that is going to do nothing. You could make it virtual instead but, again, is it really necessary?

Don’t get me wrong, I don’t mind this at all. Just being the devil’s advocate here

[Timothy_McCune]

I am also not a big fan of forcing derived classes to implement a method or property when the derived class doesn’t need it. Given the GrenadeAction you mentioned it would seem counter intuitive to do so. However, forcing 1 or 2 of a dozen Actions to implement an empty method doesn’t seem any more pointless than passing disregarded data via an EventHandler delegate(Object reference and empty EventArgs) to its subscribers, just to have them turn around and grab data via singleton instance from the very sender that invoked the delegate. But then again, I still have several sections left in the course, so hopefully that will be part of the refactoring he keeps mentioning.

[Brian_Trotter]

I tend to be a fan of abstract methods, though as Bixarrio notes, they can sometimes pigeonhole a class forcing it to implement methods it may not need…
In this case, GrenadeAction could simply implement

protected override PerformAction() {}

to satisfy the contract.