Оцените на правильность кода и общий стиль написания

@Eva Rosalene · Регистрация: 06.01.2013

Студворк — интернет-сервис помощи студентам

Приветствую всех

В кои-то веки решил начать изучать Java, наткнулся на некий курс, в котором подробно разбирается MVC, а параллельно пишется некая реализация морского боя - ну, конечно же, поэтапно.

Самое первое же простенькое задание, слава богу, затруднений не вызвало, однако хотелось бы услашать корректировки, которые сделали бы код совсем идеальным. Надеюсь на ваш скилл.

Формулировка задания

Написать программу, которая будет:
1. Запрашивать его имя игрока
2. Приветствовать его.
3. Задавать константы, так или иначе закреплённые за морским боем.
4. Описать отдельный класс корабля с ~~блекджеком и~~ координатами и размером.
5. Создать необходимое количество кораблей и вывести лог их создания.
6. Поздравить победителя, выбранного заглушкой в виде рандомогенератора.

Файл BattleShips.java

Java
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
import java.util.Scanner;
 
class BattleShips {
  public static int areaSize = 10, 
                    shipCount = 10;
  public static int[] shipsPerSize = {4, 3, 2, 1};
  
 
  public static void main (String args[]) {
    Scanner s = new Scanner(System.in);
    Ship[] ships = new Ship[10];
  
    String[] players = new String[2];
    System.out.print("[1] What is your name? > ");
    players[0] = s.nextLine();
    players[1] = "Computer";
    
    for ( int i = 0; i < 4; ++i ) {
      ships[ i ] = new Ship( i + 1, i + 1, i + 1, i + 1);
      printShip(ships[i], i + 1);
    }
    
    for ( int i = 4; i < 7; ++i ) {
      ships[ i ] = new Ship( i + 1, i + 1, i, i + 1);
      printShip(ships[i], i + 1);
    }
    
    for ( int i = 7; i < 9; ++i ) {
      ships[ i ] = new Ship( 1, 10 - i, 3, 10 - i);
      printShip(ships[i], i + 1);
    }
    
    ships[ 9 ] = new Ship( 10, 10, 7, 10);
    printShip(ships[9], 10);
    
    System.out.println("\nPlayer " + ( Math.random() > 0.5 ? players[0] : players[1] ) + " won!");
  }
  
  public static void printShip(Ship i, int number) {
    System.out.println("Ship " + number + " created" + 
               " ( " + i.coordinates[0].getX() + " ; " +
               i.coordinates[0].getY() + " ), ( " +
               i.coordinates[1].getX() + " ; " +
               i.coordinates[1].getY() + " ) [ " + i.size + " ]." 
              );
  }
};

Файл Ship.java

Java
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
import java.awt.Point;
 
public class Ship {
  public int size;
  public Point[] coordinates; 
  public boolean status = false;
  
  // Пока Public, дабы не писать аксессоры.
 
  Ship ( int ax, int ay, int bx, int by ) {
    if ( ax > 10 || ax < 1 || ay > 10 || ay < 1 || bx > 10 || bx < 1 || by > 10 || by < 1) return;
  
    int xDiff = Math.abs( bx - ax );
    int yDiff = Math.abs( by - ay );
    
    if ( xDiff != 0 && yDiff != 0) return;
    if ( xDiff == 0 ) {
      if ( yDiff > 3 ) return;
      size = yDiff + 1;
    } else {
      if ( xDiff > 3 ) return;
      size = xDiff + 1;
    }
    
    coordinates = new Point[2];
    coordinates[0] = new Point(ax, ay);
    coordinates[1] = new Point(bx, by);
    
    status = true;
    return;
  }
};

@_Den_ · 15.10.2014, 20:46

FraidZZ, А вы с ООП знакомы?

@Eva Rosalene · 15.10.2014, 20:54 **[ТС]**

_Den_, немного =) Что-то совсем сильно не так?)

Добавлено через 7 минут
Хм, только что осознал, что по старой быдлокодерской традиции-привычке описал основной код в main.

@_Den_ · 15.10.2014, 20:55

Ну на счет public полей увидел коммент, да, надо сделать их private. Я не сильно вникал в код, но похоже, что программа совсем не расширяемая, много "магических чисел", которым желательно дать имя, чтобы читабельность повысить, конструктор нужен для инициализации, вы запихнули туда много логики, которую нужно вынести, return в конструкторе это как-то странно тоже, вы прерываете инициализацию в конструкторе условием

Java
1
if ( ax > 10 || ax < 1 || ay > 10 || ay < 1 || bx > 10 || bx < 1 || by > 10 || by < 1) return;

так не делается. Ну это как минимум

@Eva Rosalene · 15.10.2014, 20:59 **[ТС]**

_Den_, а можно возыметь наглости и спросить, как всё же принято валидировать данные, передаваемые конструктору? Бросать эксепшен, как полагается?

Добавлено через 48 секунд
Логику вынесу в модель, как только пойму, что там должно быть

ТЗ-то пока тестовое.

@_Den_ · 15.10.2014, 21:14

Я бы создал отдельный класс "Координаты", каждый игрок имел бы свой объект с парой x,y, посылал бы его в Ship. Например

Java
1
Coordinates c = new Coordinates(x,y);

Можно бросить эксепшн, можно и if написать, но теперь это будет проверка двух чисел, а не 4-ех. Думаю есть еще более правильное решение, сразу сложно придумать

@Eva Rosalene · 15.10.2014, 21:25 **[ТС]**

_Den_, я не про то. На каком этапе принято проверять верность переданных в конструктор данных?

@_Den_ · 15.10.2014, 21:55

FraidZZ, На этапе ввода этих данных. Я не учитель конечно

сам учусь, но сделал бы как-то так

Java
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
import java.util.Scanner;
 
public class Ship {
 
    public Ship(Coordinates c){}
    
    public static void main(String[] args) {
        
        Scanner sc = new Scanner(System.in);
        int x=0,y=0;
 
        //повторяем ввод пока он будет не верен
        do{
            System.out.println("Input coordinates(1-10):");
            x = sc.nextInt();
            y = sc.nextInt();
        }while(!((x>=1 && x<=10) && (y>=1 && y<=10)));
        
        Coordinates c = new Coordinates(x, y);
        System.out.println(c);
        
        Ship ship = new Ship(c);
    }
 
}
 
class Coordinates{
    
    private int x;
    private int y;
    
    public Coordinates(int x, int y){
        this.x = x;
        this.y = y;
    }
 
    @Override
    public String toString() {
        return "Coordinates(" + x + "," + y + ")";
    }
}

@turbanoff · 15.10.2014, 21:59

FraidZZ, ставьте final у полей всегда, когда это возможно.
У статических - обязательно.

@Eva Rosalene · 15.10.2014, 22:08 **[ТС]**

turbanoff, _Den_, благодарю. Учту, в общем.

@lemegeton · 15.10.2014, 23:51

Валидатор надо бы делать отдельным классом.
Например, можно попробовать так.

Java
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
import java.util.ArrayList;
import java.util.List;
 
enum  ShipDirection {
    HORIZONTAL,
    VERTICAL
}
 
class Ship {
 
    private final int x;
    private final int y;
    private final int size;
    private final ShipDirection direction;
 
    public Ship(int x, int y, int size, ShipDirection direction) {
        this.x = x;
        this.y = y;
        this.size = size;
        this.direction = direction;
    }
 
    public int getX() {
        return x;
    }
 
    public int getY() {
        return y;
    }
 
    public int getSize() {
        return size;
    }
 
    public ShipDirection getDirection() {
        return direction;
    }
 
    @Override
    public String toString() {
        return "Ship{" +
                "x=" + x +
                ", y=" + y +
                ", size=" + size +
                ", direction=" + direction +
                '}';
    }
}
 
class Field {
    private final List<Ship> shipList = new ArrayList<>();
    private final Ship[][] field = new Ship[10][10];
 
    public void addShip(Ship ship) {
        shipList.add(ship);
        mapShip(ship);
    }
 
    private void mapShip(Ship ship) {
        if (ship.getDirection() == ShipDirection.HORIZONTAL) {
            for (int i = ship.getX(); i < ship.getX() + ship.getSize(); ++i) {
                field[ship.getY()][i] = ship;
            }
        } else {
            for (int i = ship.getY(); i < ship.getY() + ship.getSize(); ++i) {
                field[i][ship.getX()] = ship;
            }
        }
    }
 
    public List<Ship> getShipList() {
        return shipList;
    }
 
    public Ship[][] getField() {
        return field;
    }
}
 
class ShipValidator {
 
    private final Ship ship;
    private final Ship[][] field;
    private final boolean valid;
    private final List<String> messages;
 
    public ShipValidator(Ship ship, Ship[][] field) {
        this.ship = ship;
        this.field = field;
        this.messages = new ArrayList<>();
        this.valid = validate();
    }
 
    public List<String> getMessages() {
        return messages;
    }
 
    public boolean isValid() {
        return valid;
    }
 
    private boolean validate() {
        if (ship.getX() < 0 || ship.getY() < 0 || ship.getX() > 9 || ship.getY() > 9 ||
            (ship.getDirection() == ShipDirection.HORIZONTAL && (ship.getX() + ship.getSize()) > 9) ||
            (ship.getDirection() == ShipDirection.VERTICAL && (ship.getY() + ship.getSize()) > 9)) {
            messages.add("ship cannot be placed outside of field");
            return false;
        }
        // тут надо поискать пересечения с существующими кораблями и расположение "вплотную"
        if (ship.getDirection() == ShipDirection.HORIZONTAL) {
            int lastX = ship.getX() + ship.getSize();
            for (int x = ship.getX() > 0 ? ship.getX() - 1 : 0; x < (lastX < 9 ? lastX + 1 : 9); ++x) {
                for (int y = ship.getY() > 0 ? ship.getY() - 1 : 0; y < (ship.getY() < 9 ? ship.getY() + 1: 9); ++y) {
                    if (field[y][x] != null) {
                        messages.add(String.format("ship intersects or too close to ship '%s'", field[y][x]));
                        return false;
                    }
                }
            }
        } else {
            int lastY = ship.getY() + ship.getSize();
            for (int x = ship.getX() > 0 ? ship.getX() - 1 : 0; x < (ship.getX() < 9 ? ship.getX() + 1 : 9); ++x) {
                // x < (lastX < 9 ? lastX + 1 : 9); ++x) {
                for (int y = ship.getY() > 0 ? ship.getY() - 1 : 0; y < (lastY < 9 ? lastY + 1: 9); ++y) {
                    if (field[y][x] != null) {
                        messages.add(String.format("ship intersects or too close to ship '%s'", field[y][x]));
                        return false;
                    }
                }
            }
        }
        return true;
    }
}
 
public class Tutorial {
 
    private final static Field field = new Field();
 
    public static void addShip(Ship ship) {
        final ShipValidator validator = new ShipValidator(ship, field.getField());
        if (validator.isValid()) {
            field.addShip(ship);
        } else {
            System.out.println(validator.getMessages());
        }
    }
 
    public static void main(String[] args) {
        addShip(new Ship(0, 3, 4, ShipDirection.HORIZONTAL));
        addShip(new Ship(2, 0, 2, ShipDirection.VERTICAL));
    }
}

Новые блоги и статьи Все статьи Все блоги /
Первый деплой lagorue 16.01.2026 Не спеша развернул своё 1ое приложение в kubernetes. А дальше мне интересно создать 1фронтэнд приложения и 2 бэкэнд приложения развернуть 2 деплоя в кубере получится 2 сервиса и что-бы они. . .	Расчёт переходных процессов в цепи постоянного тока igorrr37 16.01.2026 / * Дана цепь постоянного тока с R, L, C, k(ключ), U, E, J. Программа составляет систему уравнений по 1 и 2 законам Кирхгофа, решает её и находит токи на L и напряжения на C в установ. режимах до и. . .	Восстановить юзерскрипты Greasemonkey из бэкапа браузера damix 15.01.2026 Если восстановить из бэкапа профиль Firefox после переустановки винды, то список юзерскриптов в Greasemonkey будет пустым. Но восстановить их можно так. Для этого понадобится консольная утилита. . .	Изучаю kubernetes lagorue 13.01.2026 А пригодятся-ли мне знания kubernetes в России?
Сукцессия микоризы: основная теория в виде двух уравнений. anaschu 11.01.2026 https:/ / rutube. ru/ video/ 7a537f578d808e67a3c6fd818a44a5c4/	WordPad для Windows 11 Jel 10.01.2026 WordPad для Windows 11 — это приложение, которое восстанавливает классический текстовый редактор WordPad в операционной системе Windows 11. После того как Microsoft исключила WordPad из. . .	Classic Notepad for Windows 11 Jel 10.01.2026 Old Classic Notepad for Windows 11 Приложение для Windows 11, позволяющее пользователям вернуть классическую версию текстового редактора «Блокнот» из Windows 10. Программа предоставляет более. . .	Почему дизайн решает? Neotwalker 09.01.2026 В современном мире, где конкуренция за внимание потребителя достигла пика, дизайн становится мощным инструментом для успеха бренда. Это не просто красивый внешний вид продукта или сайта — это. . .

@_Den_ 161 / 157 / 29 Регистрация: 21.09.2013 Сообщений: 487 Записей в блоге: 2
	15.10.2014, 20:46
	FraidZZ, А вы с ООП знакомы? 0

@Eva Rosalene Модератор 5241 / 2115 / 416 Регистрация: 06.01.2013 Сообщений: 4,846
	15.10.2014, 20:54 [ТС]
	_Den_, немного =) Что-то совсем сильно не так?) Добавлено через 7 минут Хм, только что осознал, что по старой быдлокодерской традиции-привычке описал основной код в main. 0

@Eva Rosalene Модератор 5241 / 2115 / 416 Регистрация: 06.01.2013 Сообщений: 4,846
	15.10.2014, 20:59 [ТС]
	_Den_, а можно возыметь наглости и спросить, как всё же принято валидировать данные, передаваемые конструктору? Бросать эксепшен, как полагается? Добавлено через 48 секунд Логику вынесу в модель, как только пойму, что там должно быть ТЗ-то пока тестовое. 0

@Eva Rosalene Модератор 5241 / 2115 / 416 Регистрация: 06.01.2013 Сообщений: 4,846
	15.10.2014, 21:25 [ТС]
	_Den_, я не про то. На каком этапе принято проверять верность переданных в конструктор данных? 0

@turbanoff 4094 / 3828 / 745 Регистрация: 18.05.2010 Сообщений: 9,331 Записей в блоге: 12
	15.10.2014, 21:59
	FraidZZ, ставьте final у полей всегда, когда это возможно. У статических - обязательно. 1

@Eva Rosalene Модератор 5241 / 2115 / 416 Регистрация: 06.01.2013 Сообщений: 4,846
	15.10.2014, 22:08 [ТС]
	turbanoff, _Den_, благодарю. Учту, в общем. 0