Java OOP: how to create objects properly

Question

Im creating a simple bowling game using OOP, and i want to have a class for each bowl, a Frame class consisting of two bowls, and a Game class consisting of ten frames.

At the moment i have something like this

Bowl.java

public class Bowl {

int bowlScore = 0;

public Bowl(int pinsKnocked){
    bowlScore = pinsKnocked;
}
}

Frame.java

public class Frame{

int firstScore;
int secondScore;
public Bowl firstBowl;
public Bowl secondBowl;

public Frame (){
    firstBowl = new Bowl(0);
    secondBowl = new Bowl(0);
}

public Frame (int firstScore, int secondScore){
    firstBowl = new Bowl(firstScore);
    secondBowl = new Bowl(secondScore);
} 

Game.java

public class Game {

int totalScore;
public Frame firstFrame;
public Frame secondFrame;
...
    public Frame tenthFrame;

public Game(){
    firstFrame = new Frame();   
}

public Game(Frame f){
    firstFrame = f;
}

Is this the correct way of using OOP features or how would I be able to improve this?

Answer 1

There is not a 100% correct way for designing a Bowling game; there are many solutions which will work, and even more that won't work.

What you need is a solution that will work well for your, and your goals.

If you want to display a score, then I suggest you start with a getScore() method. If you want to display the Winners, start with a displayWinners() method.

Eventually you will find that these methods naturally bind to various nouns. For example, you might start off with getScore() being attached to a Game object, but then realize that this unnaturally means your game can only have one score. If this occurs, you would then move the getScore() to a Player object, and have the game maintain one or more players.

If you are working with a method that naturally belongs somewhere else, there are a number of hints in your code that will guide you. The most obvious hint is that a method seems to be particularly interested in another object's data, even over it's own data. In the above example, a getScore() in a game object is overly interested in a Player's frames, balls, etc.

To gain skills and direction in how to safely move code from one place to another, I recommend reading Martin Fowler's Refactoring Book.

An excellent example, using exactly your problem is demonstrated here.

Good luck, and after a while you will be able to skip some of this process due to acquired skill; however, when learning for the first time, it is a good idea to not skip such exploratory steps (start at the beginning).

PS. Remember that your code will only be certain to do what you test it to do, if you are not familiar with test driven development, it might be a good idea to look into it (hint, this is a massive understatement).

Answer 2

As Edwin has mentioned they are lot of ways to model Bowling game. However here I'll list possible corrections to your code to improve it.
There are lot of things to improve here
1. score attribute is applicable only to Bowl class. so remove score attributes from Frame and Game.
2. bowlScore in Bowl should be private and provide getter method for it. You will
3. now Frame class should be like:

public class Frame{
    private Bowl firstBowl;
    private Bowl secondBowl;

    public Frame (int firstScore, int secondScore){
        firstBowl = new Bowl(firstScore);
        secondBowl = new Bowl(secondScore);
    }
    public int getFrameScore(){
       return (firstBowl.getScore()+secondBowl.getScore());
    }
} 

4.In Game class, you have constructor where you are passing only one frame? One game by one player consist of 10 frames. Also its not good idea to use 10 variables for frames. We hav java collections for this purpose. You can use list.

public class Game {
    private java.uti.List<Frame> frames;
    public Game(List<Frame> frames){
      this.frames = frames;
    }
    public getGameScore(){
      // loop goes here to sum up scores from all frames
      //sum = sum+frames.get(i);
    }
}

}

5.Also this modeling is valid if you are assuming that this game will be played by only one player. For multiple players, above Game class actually becomes Player class and you will have to create new Game class.

Answer 3

I'd be inclined to drop the multiple instances of Bowl. If you find yourself in this situation, ask yourself - how would I deal with 100 instances of Bowl? Consider the ten instances of Frame that you would have to create and maintain during the lifespan of the game. Keeping multiple instances is not a good idea unless you need multiple instances for some business logic.

public class Bowl {

    private int bowlScore;

    // Use a no-argument constructor
    public Bowl() {
        this.bowlScore = 0;
    }

    public void setBowlScore( int score ) {
        this.bowlScore = score;
    }

    public int getBowlScore() {
        return this.bowlScore;
    }
}

For the Frame class,

public class Frame {

    private int frameScore;
    private Bowl bowlArray[];

    public Frame() {
        this.frameScore = 0;
        this.bowlArray = new Bowl[2];
    }

    public void setScoreForFirstBowl( int score ) {
        this.bowlArray[0] = score;
        this.frameScore += score;
    }

    public void setScoreForSecondBowl( int score ) {
        this.bowlArray[1] = score;
        this.frameScore += score;
    }

    public void setFrameScore( int score ) {
        this.frameScore = score;
    }

    public int getFrameScore() {
        return this.frameScore;
    }

    // this should not be used, left in for completeness
    public Bowl[] getBowlArray() {
        return this.bowlArray;
    }
}

And for Game

public class Game {

     private int gameScore;
     private ArrayList<Frame> gameFrames;

     public Game() {
         this.gameScore = 0;
         this.gameFrames = new ArrayList<Frame>();
     }

     /* There are many ways of implementing the game logic. Here is an example.
        You will have to complete the rest :) */

     // @frame frame object with bowl data that is appended to list
     public void frameCompleted(Frame frame) {
         this.gameScore += frame.getFrameScore; // I assume this is what you want to do. Change if not
         this.gameFrames.add(frame);
     }

     /* The method written above can also be implemented by passing integer values
        for the score, bowl number and frame number. However, this would not be very
        OOP friendly. Essentially, this is 'Encapsulation' of the Frame data into the 
        Frame object. */

     // Add getters and setters for gameScore and gameFrames
}

See the ArrayList documentation for examples on further use. You can replace it with an array if you don't want to use it. I included it simply to showcase the list ability.

Read more on Encapsulation here and here.

You will notice I have not taken 'strikes' into account in the Bowl or Frame classes. This is because it is a special case that needs to catered for and so I have left it for you to implement. My understanding is if you roll a strike on the first bowl you don't get a second bowl.