• Post Reply Bookmark Topic Watch Topic
  • New Topic

How to move methods to another class and use inheritance properly  RSS feed

 
Conor Niall
Greenhorn
Posts: 19
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hi guys,

I am developing a game (it will be computer player v human player) for one of my projects, and I am struggling with how to sue inheritance and abstract classes properly. At the moment I have 3 classes - a Game class, GUI class, and Main class (this is only used to call the main() method which invokes the GUI() constructor) - and everything is working correctly. I now want to move the code relating to the computer player to its own dedicated class - a ComputerPlayer class - which will eventually become abstract as I will add 2 subclasses to it which will represent 2 different strategies (at the moment I have only developed a simple strategy in the Game class).

I am having trouble understanding where to reference the Computer Player class so that I can call it's methods, without getting NullPointerExceptions or StackOverFlow errors. How do I instantiate the ComputerPlayer, Game, and GUI classes so that I only ever have one instance of each?
Do I reference the classes in the constructor or an instance variable? And in what classes would I do this?

Currently I am invoking a GUI from the main() method. Then referencing a Game object as an instance variable within the GUI class. It all works fine so far, but I just can't figure out where the ComputerPlayer class/object would be created.



Thanks
 
Junilu Lacar
Sheriff
Posts: 11494
180
Android Debian Eclipse IDE IntelliJ IDE Java Linux Mac Spring Ubuntu
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
It sounds like you've got some pretty good separation, at least better than what we normally see. Where do you have the methods that you think should belong to a separate Player class?  Based on what you describe, I would think that you're looking to have a hierarchy that looks something like this:

Where a Player is the superclass of ComputerPlayer and HumanPlayer.

Why are you concerned about NPE and SOE? And why are you concerned that there would be more than one instance of the classes that you mentioned?
 
Campbell Ritchie
Marshal
Posts: 56599
172
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
And what is the difference between a computer player and a human player? That difference is important, because without a difference you might be better off not having subclasses at all.
 
Conor Niall
Greenhorn
Posts: 19
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hi Junilu,

The methods that I think should belong to a separate ComputerPlayer class are currently stored in the Game class. For example here is a method to make a Computer Move:



As you can see all of the above methods that make up the computerMove() are also stored in the Game class.

My Hierarchy should look like:



As the method for myMove or humanMove is currently stored in the Game class, and I would like to keep it here rather than designing an extra class to store this.

At the moment my first step is just to move the computerMove() method from Game class into ComputerPlayer class (and leaving all it's submethods for now) but in order to call the game.generateRandom() and game.checkGameStatus() methods, etc., from the ComputerPlayer class where (and how) do I create a reference to the Game class within the ComputerPlayer class so that it knows where the method is stored?

I was worried about NPE and SOE's because I kept getting these errors when I tried to implement these changes initially and thought the NPE was because one my objects wasn't initialized properly, and then the SOE was because I had created multiple instances of one class.
 
Conor Niall
Greenhorn
Posts: 19
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Campbell Ritchie wrote:And what is the difference between a computer player and a human player? That difference is important, because without a difference you might be better off not having subclasses at all.


The difference is the humanPlayer (Me) will choose numbers in the game via button clicks in the GUI. Whereas the computerPlayer will eventually have 2 subclasses for 2 different strategies. At the moment only one strategy exists - the computerPlayer will randomly pick an available number automatically without any interaction from me.

They both have ArrayLists to store their chosen numbers.
They both have counts to keep track of the sum of their numbers.
Everything else is the same, apart from how they carry our their move() methods
 
Junilu Lacar
Sheriff
Posts: 11494
180
Android Debian Eclipse IDE IntelliJ IDE Java Linux Mac Spring Ubuntu
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Again, good job on trying to keep things high level. I was surprised to see that your computerMove() method is a very Composed method. That's a good thing and worth the effort if only for the fact that it makes the design smells very easy to see. I like that the names of your methods are also expressive of what they do. This too helps make the design smells easy to see.

The design smell is that you're making the computer move include actions that any player should not be made responsible for performing. Changing players is a game responsibility. It relates to the game flow. The method checkGameStatus() is also probably a Game responsibility. The indicator there is the name of a class in the method name. That probably means that it should just be a checkStatus() method of the Game class which other Game methods call or it can also be called as game.checkStatus().

With this in mind, review the other methods called by computerMove() and see if you can tell which ones are overstepping the bounds of a Player's responsibility.

Also, check out the Strategy Pattern. Instead of a hierarchy of players, you can probably just have one Player class into whose instances you can inject different Strategy objects.

or you could have this

The argument to the Player constructor is an object that implements a Strategy interface. The auto strategies perform calculations to determine the player's actions. The interactive one would get input from the human player before completing the Player object's actions.
 
Junilu Lacar
Sheriff
Posts: 11494
180
Android Debian Eclipse IDE IntelliJ IDE Java Linux Mac Spring Ubuntu
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I wrote:I was surprised to see that your computerMove() method is a very Composed method.

Just in case that's sending mixed signals to you, I meant that as "a pleasant surprise". We don't see that from many people who post questions here. 
 
Conor Niall
Greenhorn
Posts: 19
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hi Junilu, thanks for that. At the moment I'm only moving the computerMove() method to the ComputerClass and I am calling it's sub-methods by prefixing them with a Game object e.g. game.generateRandom() or game.changeGameStatus(). I will figure out if these methods relate to the Game logic or the Player class later, after I get this way working first.
The problem I'm having at the moment is that I am getting a NullPointerException at game.generateRandom() and if I debug it I can see that at this point the Game game = null and I can't understand why.


This happens right at the start of my program. The order goes like this:



This calls the GUI constructor which looks like:


And then at this point the compPlayer.compMove() looks like this:



Can you please help me to figure out why Game game = null at this point? And/Or advise how I should make reference to the other classes in each of my classes so that their methods can be accessed?

Thanks
 
Knute Snortum
Sheriff
Posts: 4288
127
Chrome Eclipse IDE Java Postgres Database VI Editor
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I you just need any instance of Game, I would create it in the constructor.  If you need a specific instance of Game, I'd pass it as an argument to the constructor.
 
Conor Niall
Greenhorn
Posts: 19
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Knute Snortum wrote:I you just need any instance of Game, I would create it in the constructor.  If you need a specific instance of Game, I'd pass it as an argument to the constructor.


In what constructor would you create the instance of Game? In the ComputerPlayer constructor? And then remove where it has been created in the other class?
 
Junilu Lacar
Sheriff
Posts: 11494
180
Android Debian Eclipse IDE IntelliJ IDE Java Linux Mac Spring Ubuntu
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
If that's the only constructor you have for the ComputerPlayer class, then the game field will be initialized to null by default when you instantiate a ComputerPlayer.  You'll get an NPE unless you set the game field to reference a Game object before you call the compMove() method.

The dependencies between your classes still need to be teased apart. It's not a good design to have your GUI class be responsible for instantiating a Game as it's doing on line 8. It shouldn't instantiate a ComputerPlayer either. The Game object is the most logical entity to assign the responsibility of controlling the main flow of execution as well as orchestrating the actions of the player objects and dictating when the GUI refreshes itself.  So instead of instantiating a Game, the GUI should accept a Game object as its parameter in its constructor. This is called Dependency Injection.

In main(), your flow would be: create a Game. Create a GUI, passing it the Game that was just created.  This basically tells the GUI, "Hey, this is the Game that you'll be showing to the user. When the user does anything in the GUI, you need to inform the Game of what happened so that the Game can update its state. When the Game changes state, it will tell you what the updated state is so that you can adjust the display accordingly."

Do you see how that's different from how you're envisioning your program?
 
Junilu Lacar
Sheriff
Posts: 11494
180
Android Debian Eclipse IDE IntelliJ IDE Java Linux Mac Spring Ubuntu
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
There's an old-school technique called Class-Responsibility-Collaborator (CRC) cards that helps you assign responsibilities to the different classes that participate in your program. In the real-world, each person on a team will take a card, which represents a single class. Then, they will walk through the flow, using a ball or stick to represent the current point of execution. Then they will role-play what happens and how the classes "talk" to each other.  It might help if you tried that technique.  It might be weird trying to do it all by yourself but that's basically what I was doing there with the little story about what we were telling the GUI class.
 
Knute Snortum
Sheriff
Posts: 4288
127
Chrome Eclipse IDE Java Postgres Database VI Editor
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Conor Niall wrote:
Knute Snortum wrote:I you just need any instance of Game, I would create it in the constructor.  If you need a specific instance of Game, I'd pass it as an argument to the constructor.


In what constructor would you create the instance of Game? In the ComputerPlayer constructor? And then remove where it has been created in the other class?

The code below does not "connect" the class Game and ComputerPlayer, it only declares that a variable "game" is of type Game.  It is initially null.

So somewhere you need game = new Game(); -- where depends on your design.  I say this just to clear up your confusion, not to recommend that you put specific code anywhere.  Junilu has some good suggestions about design.
 
Junilu Lacar
Sheriff
Posts: 11494
180
Android Debian Eclipse IDE IntelliJ IDE Java Linux Mac Spring Ubuntu
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
So, if we were to CRC role-play with a ball, I'd say "I have the ball, I'm going to instantiate a Game object. You, there Conor, you're that new Game object. I just called your constructor, so here's the ball." (pass the ball to you)  Then you say, "Ok, I'm a new Game object. To initialize myself, I need to create a couple of Player objects. Then I'll create the variables and data structures that I need to be able to track the overall status and progress of the game. When I'm done with all that, I'm ready to start and I hand control back to Main()." Conor passes the ball back to me. Then I say, "Ok, I have a Game that's ready to go, I just need to create a GUI so the user can interact with the Game and he can see what's happening. So, I'll create a new GUI and pass it a reference to the Game object we just created. Hey, Conor, pretend you're the GUI now. I'm going to call your constructor, passing you a reference to the Game that was just created. Go ahead and initialize yourself with it." (passing the ball to Conor, who now assumes the role of the GUI class).  And so on.

There will be some points in this "play" where someone should say, "Hey, are you sure *you* should be the one who does that stuff? Shouldn't that be XXX over here who should do that, since he has most of the information needed to accomplish that task?"

And so on.
 
Conor Niall
Greenhorn
Posts: 19
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Junilu Lacar wrote:If that's the only constructor you have for the ComputerPlayer class, then the game field will be initialized to null by default when you instantiate a ComputerPlayer.  You'll get an NPE unless you set the game field to reference a Game object before you call the compMove() method.

The dependencies between your classes still need to be teased apart. It's not a good design to have your GUI class be responsible for instantiating a Game as it's doing on line 8. It shouldn't instantiate a ComputerPlayer either. The Game object is the most logical entity to assign the responsibility of controlling the main flow of execution as well as orchestrating the actions of the player objects and dictating when the GUI refreshes itself.  So instead of instantiating a Game, the GUI should accept a Game object as its parameter in its constructor. This is called Dependency Injection.

In main(), your flow would be: create a Game. Create a GUI, passing it the Game that was just created.  This basically tells the GUI, "Hey, this is the Game that you'll be showing to the user. When the user does anything in the GUI, you need to inform the Game of what happened so that the Game can update its state. When the Game changes state, it will tell you what the updated state is so that you can adjust the display accordingly."

Do you see how that's different from how you're envisioning your program?


Hi Junilu,
Thank you so much, I think I figured it out from your previous comment. I am now passing in this in my Main class:


It seems to be working perfectly so far. Now that I have that working, I may try and add a subclass, and then follow some of your advice and distinguish what classes should be responsible for what methods.

One last quick question - are there any pros/cons to using Abstract classes as opposed to Interfaces? Would one be better than the other for my scenario?
 
Junilu Lacar
Sheriff
Posts: 11494
180
Android Debian Eclipse IDE IntelliJ IDE Java Linux Mac Spring Ubuntu
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Conor Niall wrote:One last quick question - are there any pros/cons to using Abstract classes as opposed to Interfaces? Would one be better than the other for my scenario?

Well, the obvious difference is that interfaces are not tied to a specific object hierarchy whereas abstract classes are. Abstract classes are appropriate when you have some base or default behavior that you would like to provide or enforce. When you are enforcing a common behavior in a class hierarchy, you would define at least one final method in the abstract class. Prior to Java 8 you couldn't do that with interfaces. With Java 8 default interface methods, however, it's now possible to do that. I would avoid repurposing Java 8 default interface methods to enforce a common behavior though. I don't think that's what default interface methods were meant for.
 
Campbell Ritchie
Marshal
Posts: 56599
172
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
We have a wiki about the differences between interfaces and abstract classes. As Junilu says, there were changes in Java8 which blurred that distinction.
To go beyond the dependency injection, I think you shou‍ld have your game working with a command‑line/terminal interface (as far as possible) before you even try to write a GUI for it.
 
Junilu Lacar
Sheriff
Posts: 11494
180
Android Debian Eclipse IDE IntelliJ IDE Java Linux Mac Spring Ubuntu
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Campbell Ritchie wrote:To go beyond the dependency injection, I think you shou‍ld have your game working with a command‑line/terminal interface (as far as possible) before you even try to write a GUI for it.

I absolutely agree with this recommendation. A GUI just adds more complexity and it often distracts you from the very important task of assigning responsibilities properly in your design.
 
Piet Souris
Master Rancher
Posts: 2044
75
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
That is such a cliché. It depends on the game at hand. If you look here
https://coderanch.com/t/672970/java/Chess-game-Java-AWT-Swing#3151097
you see how little effort it is to come up with a working GUI. Since it helps enormously to picture what you are doing and that it makes testing of say the complicated move engine so much easier, it is time very well spent. If the same holds for OP here I don't know, he might give it some thoughts.
 
Campbell Ritchie
Marshal
Posts: 56599
172
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I am not convinced the example with the chessboard is relevant here. The thread doesn't actually describe a game engine to use.
 
Junilu Lacar
Sheriff
Posts: 11494
180
Android Debian Eclipse IDE IntelliJ IDE Java Linux Mac Spring Ubuntu
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Campbell Ritchie wrote:I am not convinced the example with the chessboard is relevant here. The thread doesn't actually describe a game engine to use.

At the risk of sounding like an echo, I have to agree. The problem the OP in that chessboard game thread was trying to solve was the GUI itself, not how the game flowed or what program entities were involved and what their relationships and responsibilities are, as it is in this thread. Furthermore, I'm willing to bet that if we looked at the fully implemented program that did include other program entities and game flow logic in that chess thread, we'd see a lot of unnecessarily tight coupling with the GUI.
 
Junilu Lacar
Sheriff
Posts: 11494
180
Android Debian Eclipse IDE IntelliJ IDE Java Linux Mac Spring Ubuntu
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I have seen people fall into the trap that focusing too early on the input/output aspects of a program lays for a programmer's mind. In the Global Day of Code Retreat event that I facilitated this past October, there was one participant who just could not get out of the trap of wanting to focus on reading in an initial board configuration of Conway's Game of Life and displaying it. No matter how many suggestions I gave for a different starting point, he was trapped in the mindset of needing to see what the configuration was like before proceeding to define proper objects and methods. That in spite of me drawing up multiple configurations on the whiteboard and flipchart sheets on the wall.

We have numerous examples of people falling into that same trap here in these forums. Heck it's easy to fall into that trap even when you have a simple character-based interface. The complexities involved in spinning up a Swing GUI is just too much too soon if that's where you start off, IMO.
 
Campbell Ritchie
Marshal
Posts: 56599
172
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Junilu Lacar wrote:. . . The problem the OP in that chessboard game thread was trying to solve was the GUI itself . . .
It is of course quite in order to create a GUI for the sake of creating a GUI. If yo usimply make a chessboard on screen, it is possible to play chess on that board. There does not actually have to be a game engine associated with it.
 
Junilu Lacar
Sheriff
Posts: 11494
180
Android Debian Eclipse IDE IntelliJ IDE Java Linux Mac Spring Ubuntu
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
To extend an olive branch to Piet, however, I will concede that if the programmer is very disciplined and organized in his/her approach, they can still arrive at a well-factored solution even if they start with the GUI. There is merit to Piet's assertion that being able to visualize the problem helps you figure out more of the solution. The problems arise when the programmer lacks the discipline and organization needed to be successful in using this strategy. In my experience, there are way fewer programmers who have that kind of skill and discipline than those who don't.
 
With a little knowledge, a cast iron skillet is non-stick and lasts a lifetime.
  • Post Reply Bookmark Topic Watch Topic
  • New Topic
Boost this thread!