I am making a game of monopoly. Inside my game I have a Board class. The Board class has an array which contains 40 squares. Each square is a different class e.g. ChanceSquare, PropertySquare FineSquare.

Now inside the board class I want to do various things. Check three separate squares to see if they have the same owner. Or Check if we can add a house to one property depending on if another one of the same set has a house also. There are various other methods that all rely on the squares array in the board class. This means my board class is responsible for multiple things and breaks SRP because every method relies on the squares array, I find myself with 20-30 methods in this class. Ideally what I would like to do is pass this array into various other classes instantiated in the board class.

E.g. a House Class – which will have methods which rely on the squares array. Or a Mortgage Class – which will have methods which rely on the squares array.

Now instead of continuously adding methods to my board class and making it massive, can I simply pass in a property of the board class (the squares array) into other classes instantiated in the board ? This will therefore make various small classes which all rely on the same array originally instantiated as a property in the board class and passed to various other classes as a constructor and all methods of the board class can be separated into more cohesive classes?

Below is a completely watered down version of what I want to implement

class PointsManager {
  constructor(playersArray) {
  this.players = playersArray;
  }

  changePoints() {// this mehtod is taken out of the players class

    if(players.length>2){
      this.players[0].points = 100;
      console.log(this.players[0].points); // equals 100
    }
  }
}

class Players {
  constructor() {
    this.players = [
      { name: "kevin", points: 25 },
      { name: "john", points: 36 },
      { name: "robert", points: 50 },
    ];

    this.pointsManager = new PointsManager(this.players);

  }
}

const players = new Players();
players.pointsManager.changePoints();

I completely get that the changePoints method should be in a separate Player Class if it didn’t rely on the players array , but in this circumstance it does and in my game of monopoly this sort of occurrence is regular. However doing it this way means internal data from one class in another. Whilst I accept this is a common practice, this is done normally via calling a method which changes data in a class from another, not be accessing the data itself which is what we are doing here !

Is this a good practice? I feel like I have two choices.

1. keep all methods that rely on the internals of an object in an array in the same class

2. pass the array into other classes instantiated inside the outer Board class

A lot of the time I can call methods directly on the objects inside the array, but sometimes one objects behaviour depends upon the state of the other objects in the same array.

To get a professional answer, I have tried to source the internet for answer, but I did not get them.

>Solution :

Your approach may be ok as long as the players array does not have any hidden dependencies inside your Players class, so your PointsManager is free to manipulate it’s content without the risk of creating any inconsistencies. For example, there could be an attribute "sumOfAllPoints" for some reason, and the Players class could be responsible to maintain this attribute in sync with the points of the individual points of the players, in which case it would be a bad idea to let a PointsManager directly manipulate points in the player array. It is also ok when your PointsManager just reads data from the array.

Otherwise, it would be less error prone to add some public access methods to the Players class which care for consistency, pass the Players object itself to the PointsManager, and let the PointsManager access the Players only through its public interface. The drawback is that this will create a cyclic dependency between Players and PointsManager (which might be perfectly fine here or not, it depends). If the cyclic dependency starts to make trouble, you could also introduce some functional interface, to allow a PointManager to access the player’s points through something like a callback function. The drawback is, this will make the code more complicated and less readable.

When you run into this situation in a larger class (like the Board class with a square array you were initially talking about), you could either pass the square array to a House class (under the constraints mentioned above), or instead the Board object itself (assumed this provides some interface to manipulate the square array in a consistent way). Alternatively, you may consider to encapsulate the square array in a new Square class first, use that as a member of Board and then pass the Square object to the House, if this helps to prevent state inconsistencies.