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

@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));
    }
}

Новые блоги и статьи Все статьи Все блоги /
Перемещение выделенных строк ТЧ из одного документа в другой Maks 30.03.2026 Реализация из решения ниже выполнена на примере нетипового документа "ВыдачаОборудованияНаСпецтехнику" с единственной табличной частью "ОборудованиеИКомплектующие" разработанного в конфигурации КА2. . . .	Functional First Web Framework Suave DevAlt 30.03.2026 Sauve. IO Апнулись до NET10. Из зависимостей один пакет, работает одинаково хорошо как в режиме проекта так и в интерактивном режиме. из сложностей - чисто функциональный подход. Решил. . .	Автоматическое создание документа при проведении другого документа Maks 29.03.2026 Реализация из решения ниже выполнена на нетиповых документах, разработанных в конфигурации КА2. Есть нетиповой документ "ЗаявкаНаРемонтСпецтехники" и нетиповой документ "ПланированиеСпецтехники". В. . .	Настройка движения справочника по регистру сведений Maks 29.03.2026 Решение ниже реализовано на примере нетипового справочника "ТарифыМобильнойСвязи" разработанного в конфигурации КА2, с целью учета корпоративной мобильной связи в коммерческом предприятии. . . .
Автозаполнение реквизита при выборе элемента справочника Maks 27.03.2026 Программный код из решения ниже на примере нетипового документа "ЗаявкаНаРемонтСпецтехники" разработанного в конфигурации КА2. При выборе "Спецтехники" (Тип Справочник. Спецтехника), заполняется. . .	Сумматор с применением элементов трёх состояний. Hrethgir 26.03.2026 Тут. https:/ / fips. ru/ EGD/ ab3c85c8-836d-4866-871b-c2f0c5d77fbc Первый документ красиво выглядит, но без схемы. Это конечно не даёт никаких плюсов автору, но тем не менее. . . всё может быть. . .	Автозаполнение реквизитов при создании документа Maks 26.03.2026 Программный код из решения ниже размещается в модуле объекта документа, в процедуре "ПриСозданииНаСервере". Алгоритм проверки заполнения реализован для исключения перезаписи значения реквизита,. . .	Команды формы и диалоговое окно Maks 26.03.2026 1. Команда формы "ЗаполнитьЗапчасти". Программный код из решения ниже на примере нетипового документа "ЗаявкаНаРемонтСпецтехники" разработанного в конфигурации КА2. В качестве источника данных. . .

@_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