1
\$\begingroup\$

I created preliminary version of battleship game. For now it works that: The computer generates a new a board and our task is to find all ships. I emphasize that I didn't focus on looks.

If anyone has a moment please look and give me feedback. I tried to create according to MVC pattern, but I don't know if I understood this well.

GitHub

package battleship;

import java.awt.EventQueue;

import javax.swing.JFrame;

public class GameCreator {

    public static void main (String[] args)
    {
        EventQueue.invokeLater(new Runnable()
        {
           public void run()
           {

               BoardView bv = new BoardView();
               JFrame frame = new JFrame();
               BoardModel s = new BoardModel();
               BoardController c = new BoardController(bv,s);
               frame.add(bv.getGui());   
               frame.pack();
               frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
               frame.setVisible(true);
           }
        });
    }

}

package battleship;

import java.awt.BorderLayout;

import javax.swing.JFrame;
import javax.swing.JLabel;
import javax.swing.JToolBar;

public class Frame extends JFrame{

    public Frame()
    {
        add(new JLabel("GRACZ"), BorderLayout.NORTH);
        JToolBar tools = new JToolBar();
        tools.setFloatable(false);
        add(tools, BorderLayout.EAST);
        BoardView b = new BoardView();
        pack();
    }
}

package battleship;

import java.awt.Color;
import java.awt.Graphics;
import java.awt.Graphics2D;
import java.awt.Insets;
import java.awt.image.BufferedImage;

import javax.swing.ImageIcon;
import javax.swing.JButton;
import javax.swing.border.LineBorder;

public class MyButton extends JButton{

    private int row;
    private int col;
    private int z;

    public MyButton(int row, int col, int z){
        super();
        this.row=row;
        this.col=col;
        this.z=z;

        setBorder(new LineBorder(Color.DARK_GRAY));
        ImageIcon icon = new ImageIcon(
                new BufferedImage(16, 16, BufferedImage.TYPE_INT_ARGB));
        setIcon(icon);
        Insets buttonMargin = new Insets(0,0,0,0);
        setMargin(buttonMargin);

        setBackground(Color.WHITE);
    }


    public int getRow(){
        return row;

    }

    public int getColumn(){
        return col;
    }

    public int getZ(){
        return z;
    }

    public void setZ(int z){
        this.z=z;
    }

}

package battleship;

import java.awt.*;
import java.awt.event.MouseListener;
import java.util.ArrayList;

import javax.swing.*;
import javax.swing.border.Border;
import javax.swing.border.EmptyBorder;
import javax.swing.border.LineBorder;

public class BoardView {

    private JPanel gui = new JPanel(new BorderLayout(5,5));
    private MyButton[][] myButtons = new MyButton[10][10];
    private final String COLS = "ABCDEFGHIJ";
    private final int size=10;


    public BoardView(){

        drawBoard();
    }

    public void drawBoard(){

        gui.setSize(1000,400);
        gui.setBorder(new EmptyBorder(5,5,5,5));
        gui.setBorder(new LineBorder(Color.BLACK));

        JToolBar tools = new JToolBar();
        tools.setFloatable(false);
        gui.add(tools, BorderLayout.PAGE_START);
        tools.add(new JButton("New")); // TODO - add functionality!

        JPanel board = new JPanel();
        board.setLayout(new GridLayout(11,11));
        board.setBorder(new LineBorder(Color.BLUE, 1));

        gui.add(board);

        for (int i=0; i<myButtons.length; i++){
            for (int j=0; j<myButtons[i].length; j++){
                myButtons[i][j]=new MyButton(i,j,0);

            }   
        }

        board.add(new JLabel());
        for (int ii = 0; ii < 10; ii++) {
            board.add(new JLabel(COLS.substring(ii, ii + 1),
                    SwingConstants.CENTER));
        }

        for (int ii = 0; ii < size; ii++) {
            for (int jj = 0; jj < size; jj++) {
                switch (jj) {
                    case 0:
                        board.add(new JLabel("" + (ii + 1),
                                SwingConstants.CENTER));
                    default:
                        board.add(myButtons[ii][jj]);
                }
            }
        }       


    }

    public JPanel getGui(){
        return gui;
    }

    public MyButton getButton(int x, int y){
        return myButtons[x][y];

    }

    public void addListener(MouseListener mouseListener){

        for (int i=0; i<myButtons.length; i++)
            for (int j=0; j<myButtons[i].length;j++)
                myButtons[i][j].addMouseListener(mouseListener);
    }

    public void paintShip(ArrayList<MyButton> mybuttons){
        for(int i =0; i<mybuttons.size(); i++)
            mybuttons.get(i).setBorder(new LineBorder(new Color(50, 220, 30)));
    }

    public void paintEmpty(MyButton mybutton){
        mybutton.setIcon(new ImageIcon("yellow-ball.gif"));
        mybutton.setDisabledIcon(new ImageIcon("yellow-ball.gif"));
    }

    public void paintMast(MyButton mybutton){
        mybutton.setBorder(new LineBorder(Color.RED));
    }

    public void paintX(MyButton mybutton){
        mybutton.setIcon(new ImageIcon("krzyżyk.jpg"));
        mybutton.setDisabledIcon(new ImageIcon("krzyżyk.jpg"));
    }

    public void paintStart(MyButton mybutton){
        mybutton.setIcon(new ImageIcon());

    }
    public void endGame(){
        JOptionPane.showMessageDialog(gui, "All Ships are sunken!");
    }



}

package battleship;

import java.awt.event.MouseAdapter;
import java.awt.event.MouseEvent;
import java.awt.event.MouseListener;
import java.util.ArrayList;

import javax.swing.AbstractButton;

public class BoardController {
    private BoardView view;
    private BoardModel model;
    private ArrayList<ArrayList<MyButton>> buttonsList;

    public void createButtonsList(){
        buttonsList=new ArrayList<ArrayList<MyButton>>();
        for(int i=0; i<=9; i++){
            buttonsList.add(new ArrayList<MyButton>());
        }
    }

    public BoardController(BoardView bv, BoardModel s ){

        this.view = bv;
        this.model= s;
        view.addListener(new MyListener());
        createButtonsList();

    }

    public class MyListener extends MouseAdapter{

        public void mouseClicked(MouseEvent e){

            if(e.getButton() == 1){

                if (!((MyButton)e.getSource()).isEnabled()) {
                    return;
                }

                showField((MyButton)e.getSource());    

            }
            else if(e.getButton()==3)
            {
                if(!((MyButton)e.getSource()).isEnabled() )
                    return;
                flagEmpty((MyButton)e.getSource());
            }
        }
    }

    public void showField(MyButton mybutton){


        boolean t =model.checkMove(mybutton.getRow(),mybutton.getColumn());
        if(t){
            view.paintX(mybutton);
            int iter = model.getShipNumber(mybutton.getRow(), mybutton.getColumn());
            addButtonToShip(mybutton, iter);
            if(model.isSunk(iter)){
                view.paintShip(buttonsList.get(iter));
                if(model.isWinner())
                    view.endGame();
            }

            else view.paintMast(mybutton);
        }
        else view.paintEmpty(mybutton);
        mybutton.setEnabled(false);
    }

    public void flagEmpty(MyButton mybutton){
        if(mybutton.getZ()==0){
            view.paintEmpty(mybutton);
            mybutton.setZ(1);
        }
        else if(mybutton.getZ()==1){
            view.paintStart(mybutton);
            mybutton.setZ(0);
        }
    }


    public void addButtonToShip(MyButton mybutton, int i){

        buttonsList.get(i).add(mybutton);
    }



}

package battleship;

import java.util.ArrayList;
import java.util.Iterator;
import java.util.Random;

import battleship.Field.State;

public class BoardModel {

    private static int currentAmount;
    private static Field[][] fields = new Field[10][10];
    private Random generator = new Random();
    private ArrayList<ArrayList<Field>> shapesList;
    private static ArrayList<Field> list1;
    private static ArrayList<Field> list2;

    {
    int i, j;
    for(i=0; i<10; i++){
        for(j=0; j<10; j++){
            fields[i][j]=new Field(i,j);
        }
    }
    }

    public BoardModel (){
        shapesList = new ArrayList<ArrayList<Field>>();
        shapesList.add(createShip(4));
        shapesList.add(createShip(3));
        shapesList.add(createShip(3));
        shapesList.add(createShip(2));
        shapesList.add(createShip(2));
        shapesList.add(createShip(2));
        shapesList.add(createShip(1));
        shapesList.add(createShip(1));
        shapesList.add(createShip(1));
        shapesList.add(createShip(1));

    }
    public ArrayList<Field> createShip(int mastsAmount)
    {
        list1 = new ArrayList<>();

        do {
            currentAmount = 0;
            int x = generator.nextInt(10);
            int y = generator.nextInt(10);

            if (fields[x][y].equalsState(State.Available)){

                    currentAmount=1;
                    fields[x][y].setState(State.Ship);
                    list1.add(fields[x][y]);
                    if(mastsAmount>1){
                        list2 = new ArrayList<>();
                        addMast(x,y, mastsAmount);
                    }
            }
        }while(!(list1.size()==mastsAmount));

        for(int i = 0; i<list1.size(); i++)
        {
            for(int j =list1.get(i).getX()-1; j<=list1.get(i).getX()+1; j++)
                if (j>=0 && j<=9)
                    for( int k = list1.get(i).getY()-1; k<=list1.get(i).getY()+1; k++)
                        if(k>=0 && k<=9)
                            if(!fields[j][k].equalsState(State.Ship)){

                                fields[j][k].setState(State.Unavailable);
                            }
        }

        return list1;
    }

    public ArrayList<Field> addMast(int x, int y, int mastsAmount){

        //add all potential masts to list2
        for (int i = x-1; i<=x+1 ; i++){
            if (i>=0 && i<=9)
                for(int j= y-1; j<=y+1; j++)
                    if(j>=0 && j<=9)
                        if(((i!=x && j==y) || (i==x && j!=y)) && fields[i][j].equalsState(State.Available)){

                            list2.add(fields[i][j]);
                        }
        }   

        if (list2.size()>0){

            int r = generator.nextInt(list2.size());
            list2.get(r).setState(State.Ship);
            list1.add(list2.get(r));
            currentAmount++;
            if(currentAmount<mastsAmount){
                list2.remove(r);
                addMast(list1.get(currentAmount-1).getX(),list1.get(currentAmount-1).getY(), mastsAmount);
            }
        }
        else{

            for(int i = 0; i<list1.size(); i++)
            {
                list1.get(i).setState(State.Available);
            }
        }
        return list1;
    }

    public boolean checkMove(int x, int y){

        if( fields[x][y].equalsState(State.Ship))
            return true;
        else return false;
    }

    public int getShipNumber(int x, int y){
        int i=0;
        int iter=0;
        for(Iterator<ArrayList<Field>> iterShip =shapesList.iterator(); iterShip.hasNext();){
            i++;
            ArrayList list = iterShip.next();
            for(Iterator<Field> iterField = list.iterator(); iterField.hasNext();){
                Field field = iterField.next();
                if(field.getX()==x && field.getY()==y){
                    iterField.remove();
                    iter = i-1;
                }
            }
        }
        return iter;
    }

    public boolean isSunk(int i){
        if( shapesList.get(i).isEmpty()){
            shapesList.remove(i);
            return true;
        }else return false;
    }

    public boolean isWinner(){
        return shapesList.isEmpty();
    }

}

package battleship;

public class Field {

    private int x;
    private int y;
    public enum State{
        Ship ("Ship"),
        Available("Available"),
        Unavailable("Unavailable"),
        PShip("PShip");

        private String text;
        private State(String text){
            this.text=text;
        }

        public String toString(){
            return text;
        }
    }

    private State state;

    public Field(int x, int y){
        this.x = x;
        this.y = y;
        this.state=State.Available;


    }

    public boolean equalsState(State s){
        if(this.getState()==s) return true;
        else return false;
    }
    public void setState(State s){
        this.state = s;
    }

    public State getState(){

        return this.state;
    }

    public int getX(){
        return this.x;
    }

    public int getY(){
        return this.y;
    }



}
\$\endgroup\$
1
  • \$\begingroup\$ I removed that last question because it's off-topic here to ask about suggested resources. You'll have to ask about that on a different site. \$\endgroup\$
    – Jamal
    Aug 7, 2017 at 2:19

2 Answers 2

1
\$\begingroup\$
  1. The name MyButton doesn't really say anything. What is that class made for? Make the name more descriptive.
  2. Using col, row and z isn't the greatest idea. I'd instead do x, y, z.
  3. Why do you initialize the game in EventQueue.invokeLater?
  4. You could change this code

    for (int jj = 0; jj < size; jj++) {
        switch (jj) {
            case 0:
                board.add(new JLabel("" + (ii + 1),
                    SwingConstants.CENTER));
            default:
                board.add(myButtons[ii][jj]);
        }
    }
    

    to this

    board.add(new JLabel("" + (ii + 1), SwingConstants.CENTER));
    for (int jj = 0; jj < size; jj++) {
        board.add(myButtons[ii][jj]);
    }
    

    Improving performance and readability

  5. Not really too important but I'd move JFrame frame = new JFrame() under BoardController c = new BoardController(bv,s). That way you'll have only the relevant to BoardController code above it and only JFrame related under it. This should improve readability.

  6. In BoardView#drawBoard why do you have ii and jj? You should name it i and j respectively unless you have a reason to do so, but it doesn't look like it.
  7. paintShip should accept a List. Accepting only ArrayLists unnecessarly couples your code. And you could use a for-each loop instead
  8. BoardController shouldn't use ArrayLists either as it can accept just Lists.
  9. I stopped checking the code until Field. Why doesn't your State enum use the default toString?
  10. I don't really think your equalsState method does any good. Is it really too hard to write x.getState() == someState?
\$\endgroup\$
0
\$\begingroup\$

There is some remarks abour the naming and formats of your classes. But in my opnion the major remark is about the structure of your code.

Regarding your class names, I assume that you have tried to use the MVC pattern. However, to be perfect, the controller should not know anything about the view. The controller changes the model, the model update the view, and the view use (via the user) the controller.

So when your user click on a cell, the view should invoke a method on the controller. Inside this method, the controller query and update the model (if there is a ship, marke the ship as hit). Once updated the model notify that view that he has been changed, and the view redraw itself.

This will simplify your code because the view only display cells, translate the click to a cell coordinate and call the controller. Your view can be a simple JComponent that override paintComponent to draw cells by querying the model.

The controller contains the logic, when he receive a click from the view, it verifies if there is a boat at the given location from the model. And, if there is one, he mark it as hit via the model.

The model maintains a set of boat and a boat is reponsible to maintains his location, hits and decide if he is sunk or not.

I would represent Boat as one array of Point and one array of Boolean where the boolean at x is set to true when the user click on the point at x. When all cells are true, the boat is sunk. It encapsulate everything behind some clear methods like setHitAt, isHitAt and isSunk.

With that, you remove many of the classes related to the UI and concentrate only on the logic and the model that can be tested easily.

public class GameController {
    private final GameModel model;

    public GameController(final GameModel model) {
        this.model = model;
    }

    public void onTest(int col, int row) {
        if ( model.hasShipAt(col, row) ) {
            model.setHitAt(col, row);
        }
    }
}



public class GameModel {
    private final Set<Consumer<GameModel>> observers = new HashSet<>();
    private final Set<Boat> boats = new HashSet<>();

    public GameModel() {
        // Create and set boats
    }

    public boolean hasShipAt(int col, int row) {
        return boatAt(col, row) != null;
    }

    public boolean isSunkAt(int col, int row) {
        return boatAt(col, row).isSunk();
    }

    public boolean isHitAt(int col, int row) {
        return boatAt(col, row).isHitAt(col, row);
    }

    public void setHitAt(int col, int row) {
        boatAt(col, row).setHitAt(col, row);
        notifyChange();
    }

    private void notifyChange() {
        observers.forEach(c->c.accept(this));
    }

    public void register(Consumer<GameModel> observer) {
        this.observers.add(observer);
    }       

    private final Boat boatAt(int col, int row) {
        for (Boat boat : boats) {
            if (boat.isAt(col, row)) {
                return boat;
            }
        }
        return null; // because this is private method, otherwise Optional
    }

    private class Boat {
        private final Point[] points;
        private final Boolean[] hits;

        public Boat(Point[] points) {
            this.points = points;
            this.hits = new Boolean[points.length];
            Arrays.fill(hits, false);
        }

        private boolean isAt(int col, int row) {
            for (Point point : points) {
                if (point.x == col && point.y == row)
                    return true;
            }
            return false;
        }

        public void setHitAt(int col, int row) {
            for (int i = 0;i < points.length;i++) {
                if (points[i].x == col && points[i].y == row) {
                    hits[i] = true;
                }
            }
        }

        public boolean isHitAt(int col, int row) {
            for (int i = 0;i < points.length;i++) {
                if (points[i].x == col && points[i].y == row) {
                    return hits[i];
                }
            }
            return false;
        }

        public boolean isSunk() {
            boolean sunk = true;
            for (Boolean hit : hits) {
                sunk &= hit;
            }
            return sunk;
        }
    }
}



public class GameView extends JPanel {
    private static final Rectangle CELL = new Rectangle(16, 16);
    private static final int SIZE = 10;

    private final GameController ctrl;
    private final GameModel model;

    public GameView(final GameController ctrl, final GameModel model) {
        // ..
        addMouseListener(new MouseAdapter() {
            @Override
            public void mouseClicked(MouseEvent event) {
                int col = event.getX()/CELL.width;
                int row = event.getY()/CELL.height;
                ctrl.onTest(col, row);
            }
        });
        this.model = model;
        this.ctrl = ctrl;

        model.register(m -> repaint());
    }

    // EDITED

    /* Invoked by Swing when the component must be `repaint`ed */
    protected void paintComponent(Graphics graphics) {
        Graphics2D g2d = (Graphics2D) graphics;
        g2d.setColor(Color.BLACK);
        for (int row = 0;row <= SIZE;row++) {
            for (int col = 0;col <= SIZE;col++) {
                paintCellAt(col, row, g2d);
            }
        }
    }

    private void paintCellAt(int col, int row, Graphics2D g2d) {
        Rectangle cell = new Rectangle(col * CELL.width, row * CELL.height,
            CELL.width, CELL.height);
        if ( !model.hasShipAt(col, row) ) {
            g2d.setColor(Color.BLACK);
            g2d.draw(cell);
        } else if ( model.isSunkAt(col, row) ) {
            g2d.setColor(Color.BLACK);
            g2d.fill(cell);
        } else if ( model.isHitAt(col, row) ) {
            g2d.setColor(Color.GRAY);
            g2d.fill(cell);
        } else {
            g2d.setColor(Color.BLACK);
            g2d.draw(cell);
        }
    }
}
\$\endgroup\$
5
  • \$\begingroup\$ Could you explain me what is variable observers and why did you put there? What does method notifychange do? I don't understand how does GameView know about changes. I have the problem because i see different ways to implement MVC pattern. You written" the model update the view", but elsewhere I am reading: "the controller updates model and view" Does mouseListener shoudn't be implemented in Controller? \$\endgroup\$
    – Mitchu
    Aug 10, 2017 at 12:57
  • \$\begingroup\$ observers is just a collection of other classes interested in changes of the model. notifyChange call the observers by passing the model itself. GameView register itself to the model. In fact I don't register the view istelf but use lambdas to call the repaint method each time the model changes. \$\endgroup\$
    – gervais.b
    Aug 10, 2017 at 14:25
  • \$\begingroup\$ If you implement the mouse listener inside your controller you link it to your view technology. If it is the view that call the controller, you can easily switch teh veiw technology without changing anything else. It is also more easy to test your controller because you can call the onTest and verify the interactiosn with the model. (Mvc is also described in the same direction by Microsoft msdn.microsoft.com/en-us/library/ff649643.aspx ) \$\endgroup\$
    – gervais.b
    Aug 10, 2017 at 14:39
  • \$\begingroup\$ I still can't understand how the view updates itself. Could you give me an example of 1 method? For example method that change colour of cell when I hit ship. \$\endgroup\$
    – Mitchu
    Aug 16, 2017 at 19:31
  • \$\begingroup\$ The view is a subclass of JComponent that paint itself. There is a hierarchy of painting methods that are called by Swing. You just tell Swing to repaint the component when it can via the repaint method. I override paintComponent to loop on each cell and call a custom paintCellAt(int col, int row, Graphics2D g2d). See my edited response. \$\endgroup\$
    – gervais.b
    Aug 19, 2017 at 19:30

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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