• Post Reply Bookmark Topic Watch Topic
  • New Topic
programming forums Java Mobile Certification Databases Caching Books Engineering Micro Controllers OS Languages Paradigms IDEs Build Tools Frameworks Application Servers Open Source This Site Careers Other all forums
this forum made possible by our volunteer staff, including ...
Marshals:
  • Campbell Ritchie
  • Liutauras Vilda
  • Jeanne Boyarsky
  • Devaka Cooray
  • Paul Clapham
Sheriffs:
  • Tim Cooke
  • Knute Snortum
  • Bear Bibeault
Saloon Keepers:
  • Ron McLeod
  • Tim Moores
  • Stephan van Hulst
  • Piet Souris
  • Ganesh Patekar
Bartenders:
  • Frits Walraven
  • Carey Brown
  • Tim Holloway

Basic OOP Rock Paper Scissors game. Bad OOP practice & stuck on finishing the game

 
Greenhorn
Posts: 8
1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Feeling shitty that I can't find my way to finish this, spent an hour or two trying to make it work and my OOP clearly isn't good enough.

Where I'm at is trying to compare the computer's result with the player's and output the gameWinner, but my OOP skills fell short and I can't use the method getComputerHandChoice in class Game.

Can anyone help/guide or show me how else I can do this differently, please. Just here to learn and improve.

The game consists of 3 classes:

First Class

Second class

And Third class
 
Sheriff
Posts: 13517
223
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Likes 2
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
First, Welcome to the Ranch!

Second, don't feel shitty. On one hand, it's good that you feel that way because it means you care about doing a good job. But feeling shitty is not going to make things better. Redirecting that negative energy into something positive, like asking for help and being open to suggestions is a great start.

Now, buckle up for some feedback
 
Greenhorn
Posts: 26
1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Welcome to the Ranch.

I'm a beginner myself and I can tell you right of the bat that you can't access methods implemented in the child class from the parent class. You can only do it the other way around. As it says in the Computer class signature it EXTENDS the parent (Game) class. So the extensions made in the child class are invisible to the parent class.

Furthermore why do you even extend the Computer class? What do you inherit from the parent class?

And yes, don't try not to feel shitty. Java is like that... Will make you feel like the dumbest person alive at times



 
Marshal
Posts: 6851
470
Mac OS X VI Editor BSD Linux
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Welcome to the Ranch, Denis!

Before I go lunch, first food for your brains:

public class Computer extends Game


When you extending something, in spoken language you'd say that Computer IS-A Game. Now is that really the case? Obviously not. What you may could do, is to play the game with computer (physical thing). Did you notice (I just did now myself), that computer has 2 meanings: [1] A physical thing, set of hardware; [2] An opponent, programmed to be controlled by computer. And the latter is really what you mean.

So when you think carefully about that, something is off here, right? I don't give a concrete suggestion for now, but try to think, what you could do about in this situation? Some random ideas: "you could consider renaming this class", "you could consider implementing Game interface with this class, or class with more descriptive name".

At the beginning it may look it doesn't help to solve our problem at hand in anyway, but hang on with such predictions. Give a try, it may open your eyes for some other design decisions which may or may not will have an impact on solving your current problem at hand.
 
Junilu Lacar
Sheriff
Posts: 13517
223
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Liu is right in questioning your choices for the Computer class. That class does not represent a Game as much as it does a player of the game. That is, a player can be the computer making random choices or a human entering their choices after being prompted.

The "constant" in this idea is that there's a choice being made. The "variable" is how that choice is being made.

Here's how you might capture the "constant" nature of the idea:

or

or

All the above may look different but they are essentially the same. The only difference they have really is in the names and semantics. The structure of all these are exactly the same and they will return either "R", "P", or "S". (I'm guessing you haven't learned about enumerated types yet so we'll table that for now and stick with returning a String to represent the Rock, Paper, or Scissors moves.)
 
Junilu Lacar
Sheriff
Posts: 13517
223
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
So let's say we go with the third option,

How would this interface be used? Let's experiment with some code (ideally, it would be test code but again, let's keep this simple for now)

and

These look reasonable but we have to try them out first to see how they hold up in usage.
 
Denis Marshall
Greenhorn
Posts: 8
1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Yes I'm not too sure why I extended Computer Class. At the time I thought if I was to make another class where I could create a method for Computer to generate a hand and later compare this in another to declare winner. I think basic rookie mistake?

I will try to use enums later & improve my code after finishing this one.

Still a bit confused how to structure it, but I'll play around with it some more... change a few things and get back see how it goes
 
Junilu Lacar
Sheriff
Posts: 13517
223
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Likes 2
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I'll just finish up this segment, so you have a full slice of pie to chew on.

As I said, the Move implementations I suggested seemed reasonable but we'd first have to see how they'd be used before making a firm decision to go with them. What would code that uses these classes look like? Well, maybe something like this:

The code looks ok but it doesn't read very well and it raises a few questions. What I really want to say is something like this:

But being able to write this kind of code means we have to define the Move class (or enum, as the case may be) differently.
 
Junilu Lacar
Sheriff
Posts: 13517
223
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

Denis Marshall wrote:At the time I thought if I was to make another class where I could create a method for Computer to generate a hand and later compare this in another to declare winner. I think basic rookie mistake


Don't sweat it, even people with years of programming under their belts can make the same mistakes. It goes back to understanding how to use the inheritance mechanism properly to express ideas, not to facilitate some implementation. If you're doing it for the latter reason, it's probably a mistake.
 
Denis Marshall
Greenhorn
Posts: 8
1
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Thank you for taking your time within explaining. I appreciate it a lot

I have started again and created 4 classes;

1) Main - Put it all together and run
2) GameMoves - will use to compare "playerResult" from Player class to whatever the results from Computer class will be. e.g.
3) Player
4) Computer

So far I have worked on creating my Player class, I think it's fine. However I'm not too sure how I can sort of take it from here, because I'm afraid I might get it wrong again. How does it look? Where should I take it from here?



UPDATE:

Created Computer class too and tested its output - seems to be working. Just need to do Main and GameMoves. Opinions?
 
Liutauras Vilda
Marshal
Posts: 6851
470
Mac OS X VI Editor BSD Linux
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Denis, read carefully Junilu's posts. I remember when I read similar ones by him few years back, it clicked me at some point how picking names very carefully leads you towards the code along with the problem which are easier to understand and solve.


Have a look at this one. Does really the game supposed to worry about player's name? I'm doubted. I'd naturally think it supposed to be in Computer class, but by now you figured that Player, what Junilu suggested most likely is the best name to give to this class. I called it an Opponent initially. Do you see how an opponent is much poorer name than Player? Opponent means that there is a player who plays against, which is true in most cases, but what is not, what if two players play together towards some common goal, so that gives an ability to re-think your strategy whether you really want to go that route? Have you ever heard saying that Team consists of 11 opponents? No! They say team consists of 11 players who play against 11 players. As you see naming is really fundamental thing, so when you look at your code it makes sense, you need to get it right - to name things what they actually mean, represent.

So try to clean up your current code by renaming things and putting information in the right places. I guess your design will read way nicer, it is not about the beauty though, it is about doing "things right". Whether it be your personal little project or school assignment.
 
Liutauras Vilda
Marshal
Posts: 6851
470
Mac OS X VI Editor BSD Linux
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Have a thumb up. I see you went ahead of me and done some good job already. There are things to improve though. Will pitch in later.
 
Junilu Lacar
Sheriff
Posts: 13517
223
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

Denis Marshall wrote:However I'm not too sure how I can sort of take it from here, because I'm afraid I might get it wrong again.


Learning how to program is a trial-and-error process, a string of failed experiments. The more failures you have, the more you (hopefully) learn from them, the better you (hopefully) get.

Another way to look at it is this: "Experience is what allows you to recognize a mistake when you make it again."
 
Denis Marshall
Greenhorn
Posts: 8
1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I agree & thank you

I'm confused as to how I can use playerResult from Player class & computerResult from Computer class to compare & decide a winner. Should I even bother doing it in a new class or should I just do it in main class & how?

I have tried to finish it off in main with the following below, but it didn't work. Any ideas how I can join these two classes's results and decide a winner?




error:

Exception in thread "main" java.lang.NullPointerException
at Main.main(Main.java:9)


 
Denis Marshall
Greenhorn
Posts: 8
1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

Junilu Lacar wrote:


The code looks ok but it doesn't read very well and it raises a few questions. What I really want to say is something like this:

But being able to write this kind of code means we have to define the Move class (or enum, as the case may be) differently.



I'm not sure how I can apply this to mine in my situation

I think the the next steps might be:

Creating a new class named GameMoves
Create a method setWinner which will decide who has won depending on the comparisons given
Call methods playerHand, computerHand & setWinner in main?
 
Denis Marshall
Greenhorn
Posts: 8
1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
made it work, a bit poor in main due to repetition but works

Current code:



 
Junilu Lacar
Sheriff
Posts: 13517
223
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

Denis Marshall wrote:I'm not sure how I can apply this to mine in my situation

I think the the next steps might be:
...
Create a method setWinner which will decide who has won depending on the comparisons given
Call methods playerHand, computerHand & setWinner in main?


Doesn't sound very OO to me but it's hard to make a judgment without seeing how these get translated from ideas in your head to executable code. But at face value, "setWinner which will decide who has won depending" is both mixing two ideas and backwards of OO.

Conceptually, here's what I'd imagine could be my first cut of code based on this description:

I would not use the name setWinner() for a method though because that's thinking from an "knowledge is outside the object" perspective rather from an "knowledge is inside the object" perspective. That is, "set winner" implies that someone else has determined who the winner is and you're now telling the object to remember who that was.  If you flip that and just say, "Ok, object play a round (and tell us who won)" then you're only expressing an intent and letting the object itself take care of details. That is more object-oriented.
 
Junilu Lacar
Sheriff
Posts: 13517
223
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
It doesn't make sense to define separate APIs for Player and Computer -- you're essentially wanting the same information from them: what's their move. So, to get the move from a Player object, you call getPlayerResult but to get the result from a Computer object, you call getComputerResult().  This essentially leads to a duplication of ideas in your code. That idea is "getting a result."

Duplication is a "code smell" -- something that's off and stinks. It's a duplication because the intent is the same (get a result) even though the implementations are different. You should recognize that from the names:

   // smelly: type embedded in method name
   ... = getPlayerResult();
   ... = getComputerResult();


Since the intent is the same, you should be calling the same method on different implementations. This is what polymorphism is about.

If you re-organized the code such that you could write this instead:

   // better, now the object appropriately represents the type and the method represents the intent
   ... = player.getResult();    // obtained interactively
   ... = computer.getResult();  // obtained randomly


Then the specific type of object determines the variable implementation (random vs. interactive choice) while the common intent is expressed as a polymorphic method, getResult(). That means, it doesn't matter if it's a human player or a computer player, whatever it is, you can ask it for the result and you'd get it somehow.

By the way, I would rename that method to something like getMove(), nextMove(), or simply move().

   // even better, name makes more sense
   ... = player.move();
   ... = computer.move();
 
Denis Marshall
Greenhorn
Posts: 8
1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hey Junilu, I tried to do a little code based on what you said. It's not much, but I sort of wanted to implement what you're trying to make me understand.

Just made a quick very basic text dice game to compare what two players are getting. Better?

Perhaps I could have also had something in Player class called playerWin and on conditions met when won in main class then just call player1.playerWin or player2.playerWin. Anyway, thanks for the feedbacks for today. I do feel like I understand a little more than when I started. At least, I'd like to think so :s.


 
Liutauras Vilda
Marshal
Posts: 6851
470
Mac OS X VI Editor BSD Linux
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Denis, have a cow for taking feedback positively. How else could it be, such a great stuff for learning!

Few comments from me based on your latest code.

1. Why Player class has field rollDice? Do you need to store that number? Do you need it in this class at all?
2. Why method is called setRollDice()? That doesn't even read fluently in english, does it?

How about if you'd go even further? Don't be scared to over-engineer experimenting. What if you'd create Dice class and would pass it to rollDice() method?
i.e.

How the Dice class may look? Well, it could be as simple as the content of current setRollDice() method's content.
i.e.

Can you notice how responsibilities get separated?

Now imagine somebody said, you know, from now on the dice will be a pyramid having 5 sides (crazy, isn't? requirements...), and where you'd need to go to change the implementation in your current code? In Player class, this just doesn't sound right, get it now?

I'd expect to go to change to Dice class, really where you'd expect it such implementation to be.  
 
Liutauras Vilda
Marshal
Posts: 6851
470
Mac OS X VI Editor BSD Linux
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Now, have a look at your Player class.

Does it make sense if you say to Player: "Roll dice", and the player answers you: "Give me the dice and I will".

Look once again to implementation? Does the code communicate such idea well? Yeah! Personally for me it does! Is there a better way to do that? Maybe, maybe... Is it better than was before? I think yes - so that is a step forward.

Put all pieces into the Player class, and you'll see that your Player class has now as a field only name, then one method for getting name, and another to roll the dice. Now it is a small class, easy to understand. Doesn't contain and do the stuff it shouldn't.
   
 
Liutauras Vilda
Marshal
Posts: 6851
470
Mac OS X VI Editor BSD Linux
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
If you noticed, my thoughts shifted slightly from Junilu's maybe here, and might some parts clashed, but that really isn't a problem. You need to try those ideas and see for yourself how well code reads, whether it is easy to understand. Even better, show to your friend, course mate, does he/she/they understand your code?

Once you are happy, stick to one or another approach. The good part of trying various approaches is, that you will know several ways to implementing this. Once you face different (and at the same time similar) problem in a future, you'll have better ideas in your head which approach you may go first with or which looks most promising from the information you are given.
 
Junilu Lacar
Sheriff
Posts: 13517
223
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
In Java, methods with the get...() and set..() prefixes have a very specific meaning: they are accessors and mutators, respectively. Having a pair of methods like getRolledDice() and setRolledDice() the way you defined it doesn't follow the idiomatic use of "get" and "set" prefixes, particularly the setRollDice() method, which in your case takes no parameters and yet mutates the state of an instance variable.

That's a long-winded way of saying "I probably wouldn't do it like that."

If anything, I might prefer to do it this way instead:


These are very anaemic classes, especially the Player class since all it does is keep track of the last roll it made. But the point is to see where the responsibility is assigned, a key decision when you're writing object-oriented programs. Here, the responsibility of calculating the next roll is not assigned to the Player class. I can't really explain it right now but having the "roll" responsibility in the Player class kind of smells like a mis-assigned responsibility which is why I would rather move it out of the Player class.

In fact, in a program as simple as this, I wouldn't even create a separate class to house that code and just have the main() and nextRoll() methods and the dice field as class members (static) of the Player class. Why complicate things with more files? However, to illustrate the separation of concerns, having these two sections of code in separate classes is fine.
 
Liutauras Vilda
Marshal
Posts: 6851
470
Mac OS X VI Editor BSD Linux
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Now, you may appeal and say that you really need to store the rolled dice result and now you got robbed of this ability.

Give a try going even more further, create some class which records results.

Now, this is on a worried side. Method became more complex, and instead of rolling the dice it started doing something else! So roll back, try other way.

How about:

Can you see how many posibilities out there? You really start feel of engineering when you try out different ideas.
       
       
   
 
Liutauras Vilda
Marshal
Posts: 6851
470
Mac OS X VI Editor BSD Linux
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
And I'd agree what Junilu mentioned, that it is fairly easy to over engineer and over complicate problem. But I guess it becomes more apparent when you try out various approaches. Perhaps with time one has to attempt less times in order to come up with a decent choice.
 
Denis Marshall
Greenhorn
Posts: 8
1
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Thank you!
You people are pure motivation


Never did I ever think so much how to approach something this much haha before OOP

I did shortened it down to this bit of code & hoped to have made it more clear & precise. Done this before reading your replies tbh, but yes I get the point of it being able to be done in so many ways, however it's important to do it efficiently and it's best to do it in a way that makes sense


Player is able to roll a dice, hence I thought it's most appropriate to have rollDice method here.

Dice get's rolled and so it rolls, hence I thought it's most appropriate to have roll here.
 
Liutauras Vilda
Marshal
Posts: 6851
470
Mac OS X VI Editor BSD Linux
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
@OP

Junilu has much more experience than I do, so don't take my ideas as granted, I just putted on a table few other ideas just to show how many ways are to implement this. Give a try what he suggested. After all, that's your journey. All programmers are different, in fact, they think differently. So what really makes more sense to me, might won't make that much sense to you or others. When you work in a team, you need to find compromises.
 
Bartender
Posts: 5907
57
Eclipse IDE Firefox Browser MySQL Database VI Editor Java Windows
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
You only want to create a single instance of Random, and certainly not every time you roll().
 
Junilu Lacar
Sheriff
Posts: 13517
223
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Conceptually, what you did with Player.rollDice() and Dice.roll() is on the right track. Separating concerns into more logical chunks of code is really what it's about. When logic makes sense when it's together, it makes your code more coherent and cohesive. The same thing goes with separating logic that doesn't make sense to be together.

You have also discovered the idea of "delegation" where the Player class delegates the responsibility of getting a random number to the more abstract idea represented by a call to Dice.roll()

Good job!
 
It is sorta covered in the JavaRanch Style Guide.
  • Post Reply Bookmark Topic Watch Topic
  • New Topic
Boost this thread!