1

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.

1 Answer 1

2

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.

And please do yourself a favor and stop thinking about braindead "best practices". Different solutions are all valid, with different tradeoffs, there is no "general best practice" here.

5
  • Great answer. Yes your last paragraph is true with me. What inconsistency can occur when the array has only one place in memory. E.g I change the internals of an array, it changes in every class which has it, so the sumOfAllPoints method you mentioned would still work fine right ? I often come across this issue where I want multiple classes to share an array. I have been advised against this before , but for me it makes little sense to have 20 methods instead of 5 classes which can all target the same array. Your answer is geared towards sharing the array in the right circumstances? Dec 9, 2022 at 15:55
  • how does this sound? Also can the Players class just consist of an array with a method of getArray(). Then I can pass the Players object in which would return the internal array which I could change in each different class, then that way the Players class doesn’t use this array, it just passes it into methods of other classes , this way I can keep classes short and sharp but each one needs a the same Array, otherwise if your saying there are other methods in the players class which need the array, that means I shouldn’t pass it around to due to inconsistencies then how would 1/2 Dec 9, 2022 at 22:57
  • I get the array each time to use in other classes. How would a public interface make a different for each method, this still means by Players class is going to be big. Or Is the public interface just responsible for returning the array! I’m confused 😢 Dec 9, 2022 at 22:59
  • @KevinGreetham: the comment section here is not intended to be used for extended discussions, I cannot teach you good class design and encapsulation here in two sentences. If you want to discuss about your code, take a real, working piece to codereview.SE and ask there, I think their experts might be able to help you more.
    – Doc Brown
    Dec 10, 2022 at 8:10
  • thanks very much!! I’ve learnt something! Dec 10, 2022 at 8:32

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.