0

I'm trying to create a replica of the card game "Monopoly Deal" in C# Console, for educational purposes. I have a problem with my class design that I'm not sure how to resolve. I have two classes, Game and Player. Game controls the state of the game, and contains a list of Player objects.

Currently the way it works is that the Game class controls everything. The Player is basically a container class: it has all of the cards in the player's collections. The decision-making is done by the Game class because it can see all the players, and therefore knows which moves are possible.

For example, if Player 1 wanted to steal a card from Player 2, my program would have to first verify that Player 2 has any cards to steal. Player 1 doesn't know that, because it's just a single object in a list of Player objects. So the Game class has a method which checks all possible actions that a player could make, asks the player to choose which action they want to make, and then performs that action for them.

My problem: I don't think that this is good OOP class-design. I think that I should be delegating the decision-making to the Player class, not the Game class - otherwise my Game class will become bloated with lots of methods that relate to player actions and decisions, and not the game state itself. I don't know how to achieve this, however.

Possible solutions:

  1. Create a GameState object containing just a limited view of other players' cards, and pass that to the Player object (Seems the easiest way but I guess it's creating redundant data which is a waste of memory.)
  2. Pass a reference to the whole Game object to the Player class (I could do - it's not like I'm going to program by ComputerPlayers to cheat - but doesn't feel like good OOP design either)
  3. Give up, and put all of the game logic (including the computer AI) in the Game class, and accept that Player will just be a data container, not a decision maker.
  4. Redesign my classes a different way.
  5. Something else I've missed?

Is there any obvious solution here? Thank you.

EDIT: I've attempted to trim down excessive detail in the question as it was closed for "Not enough focus". If further revisions are needed, please let me know.

Lou
  • 2,200
  • 2
  • 33
  • 66
  • I've been previously downvoted for not including enough detail in questions - I've aimed to keep this as concise as possible with only necessary information. I'd appreciate a comment if further explanation or detail is needed. – Lou Aug 09 '21 at 14:33
  • Also, despite googling I was not able to find out if there was a better terminology for "a class object contained within another class object" (AFAIK it's not an inner class or a subclass.) If someone knows the correct term could you let me know? – Lou Aug 09 '21 at 14:38
  • 2
    It's a very long and broad question. For this [in the Top 5 recurrent question](https://www.google.com/search?q=stack+overflow+c%23+from+another+form) you can use according to your [needs or opinion](https://stackoverflow.com/search?q=%5Bc%23%5D+from+another+form): singletons, static data members, events, static run method pattern, constructor with parameters, composites, aggregates, and so on... –  Aug 09 '21 at 14:38
  • 1
    Related: [Interaction between forms - How to change a control of a form from another form?](https://stackoverflow.com/questions/38768737/) • [Passing data between forms](https://stackoverflow.com/questions/4587952/) • [Passing variable between winforms](https://stackoverflow.com/questions/4247807/) • [Passing data between C# forms](https://stackoverflow.com/questions/39188704/) • [Communicate between two windows forms in C#](https://stackoverflow.com/questions/1665533/) –  Aug 09 '21 at 14:38
  • Thanks for the references - I'll see if I can glean any insights. Those are all questions about WinForm applications though - I was asking about a Console application. – Lou Aug 09 '21 at 14:41
  • 1
    State contained within a class outside of a method is either a [Field](https://learn.microsoft.com/en-us/dotnet/csharp/programming-guide/classes-and-structs/fields) or a [Property](https://learn.microsoft.com/en-us/dotnet/csharp/programming-guide/classes-and-structs/properties). – mason Aug 09 '21 at 14:43
  • 1
    @Lou A form is a class: these coding technologies and patterns are the same for any class, even if it is a form. Don't stop at that and see the concepts beyond. –  Aug 09 '21 at 14:44
  • 2
    In general, we often try to make a distinction between classes that contain application logic and classes that represent data. So if you have a Player class that represents a player, I wouldn't embed any actual logic into it other than just enough to initialize the player's properties and maintain the player's external state. – mason Aug 09 '21 at 14:46
  • Thanks @mason - and for the terminology answer. I guess I was thinking that because I'm creating a ComputerPlayer class, the intuitive place to put the decision logic for the AI's move is within the ComputerPlayer class. But maybe I should outsource the main decision making to a separate class outside of Game and Player, as Djeurissen suggests... – Lou Aug 09 '21 at 14:47
  • Another thing: If you want to prevent agents/players from cheating, have a function like Game.hideInformation(playerID) which removes everything the agent/player shouldn't see. That way you can pass the game around and hide information if necessary – Djeurissen Aug 09 '21 at 14:50
  • @Djeurissen Ah, so basically returning a copy of the Game with limited information for decision-making purposes? – Lou Aug 09 '21 at 14:52
  • 1
    Not necessarily a copy, I would suggest to split copying and hiding information between two methods. Although you often want to copy when hiding information, this might not always be the case. This way you keep yourself open for changes – Djeurissen Aug 09 '21 at 14:53
  • I'm getting pretty tired of having questions closed on this site and nobody giving an explanation for why, or what I can actually improve about the question. Could at least one close voter be kind enough to elaborate on "Needs more focus"? The context is long but the problem is clearly stated at the end of the question. – Lou Aug 09 '21 at 15:04
  • 1
    @Lou There is no need to describe all the properties, since they are not important for the question. The same is true for the code inside TakeTurn and Play, just a description of what they try to accomplish is sufficient. Lastly, your question talks too much, try to compress it to focus on the core of the question. Do not talk endlessly about potential ideas, approaches, etc. – Djeurissen Aug 09 '21 at 15:14
  • 2
    The number of (long) comments produced within minutes of this post are a clear indication this is a discussion and not a Q&A. (It looks to me like an indication the commenters don't really know how SO is supposed to work either; but that is a separate problem.) A question on SO should ideally lead to one clear and correct answer. Not a recommendation, not a best answer, but a correct answer. There is no single correct answer to this question such that other answers would be incorrect. This is still a great question; but for a discussion forum rather than SO. Great question, wrong venue. – jaco0646 Aug 09 '21 at 15:23
  • @Djeurissen and jaco0646 - Thanks for the feedback, I really appreciate people just taking the time to tell me what I can improve, rather than closing and moving on! I've attempted to make the question more concise and focussed, and have requested a re-open as I'd still be interested to hear other solutions to this problem. Any further feedback is welcomed. – Lou Aug 10 '21 at 16:35

1 Answers1

1

How about creating an action-class that represents an action that you can take in the game. The Game then just has two methods Game.verifyAction(action) and Game.executeAction(action). The Game contains all the data necessary to verify and execute actions.

Next, treat players just as data containers, meaning a player does not think it's just contained in the state. A player might contain data like it's score, it's id, etc. To actually think you have an agent class that is responsible for choosing actions. So Agent.think(Game) returns an action that the agent wants to take in this state.

Now it's easy to move the main-loop outside the Game-class, you can also replace an Agent easily with an GUI that returns a valid action once the human chooses an action.

Djeurissen
  • 377
  • 5
  • 15
  • Thanks, I think this makes sense. So you're suggesting that an Agent class would contain a Player field with the player's data and the decision-making methods, and that you'd basically pass the "Game" as an object to the Agent's decision-making methods? – Lou Aug 09 '21 at 14:50
  • 1
    Yes, also read my other comment about hiding information. By removing the decision making from the player-class you enable yourself to do much more. For example controlling an player through an GUI instead of using an Agent – Djeurissen Aug 09 '21 at 14:52
  • 1
    @Lou, this is a fine solution; but the paradigm it describes is procedural programming. Since the question is tagged as `OOP`, you should be aware that demoting your `Player` domain object to a data structure by separating its behavior is quite the opposite of OOP. – jaco0646 Aug 09 '21 at 15:02
  • Oh, thanks for letting me know @jaco0646! – Lou Aug 09 '21 at 15:03
  • 1
    Interesting, I can see how this goes agains OOP principles, but I have never seen this solution in such a way. In my experience this approach makes the design much more flexible, however it certainly comes at a cost of not being able to add new players to a game. I normally solve this by having a GameRunner class that takes as input a list of agents. – Djeurissen Aug 09 '21 at 15:09
  • How do you mean it stops you from adding new players to a game? Wouldn't you add an 'Agent' in your solution whenever you add a 'Player'? Also, it sounds like you've implemented something like this before - I don't suppose you have some code I could see that would help me get a better understanding of the type of solution you're describing? It's a little abstract to me still – Lou Aug 11 '21 at 10:58