The view should not directly affect the model, it only interacts with…#9
The view should not directly affect the model, it only interacts with…#9showman-1 wants to merge 1 commit into
Conversation
… the controller. Add the SetTarget method to the controller. The controller should not change the view directly.
temailkaev
left a comment
temailkaev
left a comment
There was a problem hiding this comment.
Проект имеет хорошую архитектуру с разделением на MVC, корректно работает базовый функционал движения робота к цели. Однако есть несколько критических проблем, связанных с потокобезопасностью и работой Swing, которые необходимо исправить перед слиянием.
| @@ -18,18 +18,22 @@ | |||
| timer.schedule(new TimerTask() { | |||
There was a problem hiding this comment.
model.update() вызывается в потоке Timer, а не в EDT (Event Dispatch Thread). При этом RobotModel.update() изменяет состояние модели, а GameVisualizer читает эти данные в EDT. Отсутствие синхронизации может привести к race conditions и некорректному отображению. Лучше обернуть вызов в EventQueue.invokeLater()
| }, 0, 50); | ||
| }, 0, 10); | ||
|
|
||
| timer.schedule(new TimerTask() { |
There was a problem hiding this comment.
Магическое число 50 (мс) лучше вынести в константу, например private static final int REDRAW_INTERVAL_MS = 50;. Это улучшит читаемость и упростит изменение частоты обновления.
|
|
||
| public class GameController { | ||
|
|
||
| private final Timer timer = new Timer("GameTimer", true); |
There was a problem hiding this comment.
Таймер создаётся, но никогда не отменяется (кроме вызова stop()). Нужно убедиться, что stop() вызывается при закрытии окна. Добавь в GameWindow обработчик закрытия
| } | ||
|
|
||
| @Override | ||
| public void paint(Graphics g) { |
There was a problem hiding this comment.
Переопределён метод paint() вместо paintComponent(). Это может нарушить работу с двойной буферизацией и привести к артефактам при перерисовке.
|
|
||
| private static void fillOval(Graphics g, int centerX, int centerY, int diam1, int diam2) { | ||
| g.fillOval(centerX - diam1 / 2, centerY - diam2 / 2, diam1, diam2); | ||
| } |
There was a problem hiding this comment.
При нечётных диаметрах центр смещается на 0.5 пикселя. Рассмотри возможность использования Math.round() или хранения координат как double с преобразованием в int только в момент рисования. Это особенно важно для точного позиционирования робота.
| coordinatesArea.setEditable(false); // Нельзя редактировать | ||
| coordinatesArea.setFocusable(false); // Нельзя получить фокус (убирает курсор) | ||
| coordinatesArea.setFont(new Font("Monospaced", Font.PLAIN, 12)); | ||
| coordinatesArea.setBackground(null); // Прозрачный фон (как у панели) |
There was a problem hiding this comment.
Установка null для фона может вызвать проблемы с отрисовкой на некоторых LookAndFeel. Лучше использовать:
| private double targetY = 100; | ||
|
|
||
| private static final double MAX_VELOCITY = 0.1; | ||
| private static final double MAX_ANGULAR_VELOCITY = 0.001; |
There was a problem hiding this comment.
Значения выглядят очень маленькими. Убедись, что единицы измерения согласованы с duration = 10 в методе update(). Возможно, MAX_VELOCITY должен быть 1.0 (пикселей за 10 мс) или duration должен быть 0.01 (секунды). Добавь комментарий с единицами измерения
| listeners.remove(listener); | ||
| } | ||
|
|
||
| private void notifyListeners() { |
There was a problem hiding this comment.
Метод вызывается из потока таймера (в update()), но слушатели могут обновлять Swing-компоненты. Это может вызвать InterruptedException или гонки данных
| public void update() { | ||
| double distance = distanceToTarget(); | ||
|
|
||
| if (distance < 0.5) { |
There was a problem hiding this comment.
Магическое число 0.5 (пикселя) лучше вынести в константу
| velocity = applyLimits(velocity, 0, MAX_VELOCITY); | ||
| angularVelocity = applyLimits(angularVelocity, -MAX_ANGULAR_VELOCITY, MAX_ANGULAR_VELOCITY); | ||
|
|
||
| if (Math.abs(angularVelocity) < 0.00001) { |
There was a problem hiding this comment.
Порог 0.000001 лучше вынести в константу ANGULAR_VELOCITY_EPSILON = 1e-6. Это улучшит читаемость и позволит легко изменить точность.
2 participants
… the controller.
Add the SetTarget method to the controller.
The controller should not change the view directly.