-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Hometask 3 #3
base: master
Are you sure you want to change the base?
Hometask 3 #3
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Поправьте комментарии по 1му заданию.
return rooms; | ||
} | ||
|
||
public void activateAlarm(String code){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Нет, это нарушение SRP, все действия с сигнализацией - внутри Alarm.
this.alarm.activateAlarm(code); | ||
} | ||
|
||
public void deactivateAlarm(String code){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Нет, это нарушение SRP, все действия с сигнализацией - внутри Alarm.
if (isAlarmEvent(event)){ | ||
if (event.getType() == ALARM_ACTIVATE){ | ||
String currentCode = Alarm.getCode(); | ||
smartHome.activateAlarm(currentCode); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Все делать через Alarm, без дома.
code = newCode; | ||
} | ||
|
||
public static String getCode(){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
У нас один код на все создаваемые инстансы сигнализации? Разве не плохо было бы у каждой сигнализации иметь свой код? Тогда надо убрать статик.
} | ||
else { | ||
// обрабатываем событие, так как тревоги еще нет | ||
EventProcessor.processEvent(smartHome, event); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Здесь состояние AlarmActivatedState, надо остановить обработку событий и дать возможность выключить сигнализацию.
Alarm alarm = smartHome.getAlarm(); | ||
|
||
if (alarm.getState() instanceof AlarmAlertState){ | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Здесь надо дать возможность выключить сигнализацию. И вообще надо отправлять смс в тревожном и активированном состояниях.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Смс до сих пор не отправляется.
} | ||
} | ||
public static boolean isSensorEvent(SensorEvent event){ | ||
return (event.getType() == DOOR_OPEN || event.getType() == DOOR_CLOSED || |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Нарушение OCP, переделайте проверку.
|
||
@Override | ||
public void activateAlert() { | ||
alarm.getState(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Здесь должен быть перевод в AlarmAlertState.
|
||
import java.util.ArrayList; | ||
import java.util.Collection; | ||
|
||
public class SmartHome { | ||
public class SmartHome{ | ||
private static Alarm alarm; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Почему Alarm - static? Она одна на все инстансы дома?
System.out.println("Got event: " + event); | ||
new LightEventProcessor(smartHome, event).processEvent(); | ||
new DoorEventProcessor(smartHome, event).processEvent(); | ||
new HallDoorEventProcessor(smartHome, event).processEvent(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
А где работает обработчик сигнализации?
else if (alarm.getState() instanceof AlarmDeactivatedState){ | ||
|
||
// сигнализация деактивирована, обрабатываем событие | ||
EventProcessor.processEvent(smartHome, event); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Здесь должен вызываться eventProcessor - поле в этом классе, а не статичный метод из интерфейса EventProcessor, это две большие разницы.
Alarm alarm = smartHome.getAlarm(); | ||
|
||
if (alarm.getState() instanceof AlarmAlertState){ | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Смс до сих пор не отправляется.
No description provided.