Java Roguelike class and subclass structure for scheduler and actors

Question

As a learning progression for JAVA inheritance I have put together a scheduler class and actor class. The scheduler class creates a list of actor objects and progresses through them calling on the actor to act().

Now my first instinct for the actor class is to pass the player actor a reference to the Scheduler so that the player can pause the scheduler when his/her turn arrives.

So within the scheduler, I have a reference to the current actor and can do the following..

actor.act( this );

this being the scheduler.

Now within the player class I can declare act as follows...

    @Override
    public int act( Scheduler queue )
    {
        //need to pause scheduler here and wait for player to act
        queue.lock();
        //wait for action from player
                     if (this.playerActionComplete)
                     {
                           queue.unlock();
                     }
    }

Is this the right OOP approach? I've already got weird stuff happening and wondering if I should persevere with this approach.

Cheers!

Answer 1

You should not persevere with this approach for three major reasons:

1) Circular dependency. Passing this as a parameter is a typical code smell.

2) Single Responsibility. An actor should act, a player should play and a scheduler should schedule. Don't let an actor take control over the scheduler. What if one of the actors forgot to put the Scheduler.unlock in a try finally block? What if you want to introduce another type of scheduler that has a totally different way of doing things?

3) Dont Repeat Yourself / Tight coupling. I am assuming there will be different kind of actors usable by the scheduler? If so, are they all going to include the same code to lock and unlock the scheduler? Or are they all required to inherit from the same abstract base class? If so, you've not repeated yourself, but tightly coupled your actor to your scheduler, making it impossible for an actor to work without a scheduler.

Why not just let the scheduler call lock and unlock around the invocation of act?