What is the best OOP solution of my problem?

Question

For a good while now, I'm trying to structure my code better and focus on using OOP concepts more and more in my code .

I have a simple question about the following scenario and how it is solved the best in the view of design pattern.

I have a game, in which a player can rob a shop. The shop is represented by the Shop object and the robbery is represented by the RobEvent. And I have a Spawner object, which handles the spawning of the securities, when someone attempts to rob a shop.

My central problem is, that I have the feeling, that the RobEvent object has too much information and functionality described below and that I could split information and functionality in more objects and for this I need the help of you!

public class Main 
{

    public static void main(String[] args)
    {

    }

    public class Shop {

        private String name;
        private Location location;
        private boolean isBeingRobbed = false;

        /*
         * Here one of the problems, should the shop have the 
         * data of the npcs, which will appear
         * in the shop when the player attempts to rob it.
         * Should it have methods to place "spawn" positions of 
         * the securities, the type, the amount etc.
         * and store it here? 
         * 
         */

        public Shop(String name, Location location){
            this.name = name;
            this.location = location;
        }

        public boolean isBeingRobbed(){
            return isBeingRobbed;
        }

        protected void setBeingRobbed(boolean bool){
            this.isBeingRobbed = bool;
        }
    }

    public class RobEvent extends Looper {

        private Shop shop;

        RobEvent(Shop shop){
            this.shop = shop;
        }

        public void start(){
            shop.setBeingRobbed(true);
        }

        @Override
        protected void logic(){
        /*
         * Big chunk of game logic
         * Spawning the securities, checking if they 
         * all appeared(they got a delay of few seconds each),
         * check if he ran away before everything 
         * spawned, check if he killed all, check if all appeared
         */
        }

        private void completed(){
            shop.setBeingRobbed(false);
        }
    }

    public class Spawner {


        /* not important things
         * 
         * just has a method to 
         * handle the spawn of a security 
         */     

        public void spawn(NPC npc){
            /*
             * spawn the npc aka security
             */
        }

    }
}

My biggest problem is, that the logic() method gets really big. It is a method, which gets looped every second by the super class.

More detail:

• The RobEvent has to know if it is currently spawning securities and do specific things,
• it has to check if all got spawned and do specific things,
• it has to check if the player ran away before it was completed and do specific things,
• it has to check if the player killed all and do specific things etc.

Most annoying is keeping track of the spawned securities, because if he runs away, they all have to be despawned. I have the feeling that this is too much information in this object and that I could for example maybe split the tracking of the spawned securities in a seperate object and just do something like tracker.despawn when the RaidEvent has a specific state.

EDIT: Code is jut a really simplification of the actual code.

Answer 1

Along with other answers I'd also consider modelling RobEvent as an actual Event (I'm not a Java dev, I don't know how to implement those in Java). When shop gets robbed, it sends around RobEvent. The Spawner object can subscribe to those events and on each RobEvent send the securities. Then, if robber runs away, shop sends RobberRunAwayEvent which all the securities (or Despawner) are subscribed to.

Answer 2

The RobEvent has to know if it is currently spawning securities and do specific things, - it has to check if all got spawned and do specific things, - it has to check if the player ran away before it was completed and do specific things, - it has to check if the player killed all and do specific things etc.

This sounds like the State-Pattern which decouples logic from state.

Quick Implementation

RobEvent needs to track a RobState. You have multiple RobState like StartSpanSecuritiesState, AllSecuritiesSpanedState, PlayerRunAwayState and more..

interface RobState {
    void handle();
}

class RobEvent extends Looper implements RobState {

    // add new member
    private RobState robState;
    private Shop shop;

    RobEvent(Shop shop) {
        this.shop = shop;
        this.robState = new StartSpanSecuritiesState();
    }

    // add new setter - gets normally called only by a `State`
    void setRobState(RobState robState) {
        this.robState = robState;
    }

    // ..

}

Each RobState knows the next RobState and set it through the setRobState of RobEvent.

class StartSpanSecuritiesState implements RobState {

    private RobEvent robEvent;

    public StartSpanSecuritiesState(RobEvent robEvent) {
        this.robEvent = robEvent;
    }

    @Override
    public void handle() {
        // span your securities
        //...

        // go to the next state
        robEvent.setRobState(new AllSecuritiesSpanedState(robEvent));
    }
}

After all the modifications the logic method could look like:

@Override
protected void logic(){
    robState.handle();
}

---

I hope it helps you! If not this time maybe in the future!
Have fun with your game:]

Answer 3

In terms of OO structure, Classes have state (their properties) and behavior (their methods). I would look at RobEvent as a behavior of Shop, i.e. move it to a method instead of being a Class.

For your logic() method becoming too long, it's good that you recognize it's too long, and that's usually an indication that it's doing too much. The simplest approach is to split it into smaller methods that each do one specific thing and get called in turn. For example, a typical game loop might look something like:

public void gameLoop(){
    while(this.gameRunning){
        this.checkUserInputs();
        this.updateGameState();
        this.updateGameDisplay();
    }
}

Shorter/smaller methods are easier to understand and maintain than excessively long methods.