Hotel power management
$begingroup$
A Hotel needs to reduce the overall power used by the equipments installed in its many floors. So the Hotel Management has installed sensors, like Motion Sensors, etc at appropriate places .I have to design a controller which takes inputs from these sensors and controls the various equipments.
- A Hotel can have multiple floors
- Each floor can have multiple main corridors and sub corridors
- Both main corridor and sub corridor have one light each
- Both main and sub corridor lights consume 5 units of power when ON
- Both main and sub corridor have independently controllable ACs
- Both main and sub corridor ACs consume 10 units of power when ON
- All the lights in all the main corridors need to be switched ON
- When a motion is detected in one of the sub corridors the
corresponding lights need to be switched ON - When there is no motion for more than a minute the sub corridor
lights should be switched OFF - The total power consumption of all the ACs and lights combined
should not exceed (Number of Main corridors * 15) + (Number of sub
corridors * 10) units of per floor. Sub corridor AC could be
switched OFF to ensure that the power consumption is not more than
the specified maximum value - When the power consumption goes below the specified maximum value
the ACs that were switched OFF previously must be switched ON
I am writing the Controller program that takes input values for Floors, Main corridors, Sub corridors and takes
different external inputs for motion in sub corridors and for each input prints out the state of all
the lights and ACs in the hotel.
So starting with Modelling the hotel part by parts.
public abstract class Equipment {
private String id;
private boolean isON;
Equipment(String id) {
this.id = id;
}
public boolean isON() {
return isON;
}
public void switchON() {
isON = true;
}
public void switchOFF() {
isON = false;
}
public void printStatus() {
String status = isON() ? "ON" : "OFF";
System.out.println("tt" + id + " " + status);
}
abstract public int getPowerConsumption();
}
public class Light extends Equipment {
public static int SPECPOWER = 5;
public Light(String id) {
super(id);
}
@Override
public int getPowerConsumption() {
return isON() ? 5 : 0;
}
}
public class AirConditioner extends Equipment {
public static int SPECPOWER = 10;
public AirConditioner(String id) {
super(id);
}
@Override
public int getPowerConsumption() {
return isON() ? 10 : 0;
}
}
public abstract class Corridor {
private String id;
private List<Light> lights;
private List<AirConditioner> airConditioners;
public Corridor(String id, List<Light> lights, List<AirConditioner> airConditioners) {
this.id = id;
this.lights = lights;
this.airConditioners = airConditioners;
}
public List<Light> getLights() {
return lights;
}
public List<AirConditioner> getAirConditioners() {
return airConditioners;
}
public int getTotalPowerConsumption() {
int result = 0;
for (Light light : lights)
result += light.getPowerConsumption();
for (AirConditioner airConditioner : airConditioners)
result += airConditioner.getPowerConsumption();
return result;
}
public void printStatus() {
System.out.println("t" + id);
for (Light light : lights) {
light.printStatus();
}
for (AirConditioner airConditioner : airConditioners) {
airConditioner.printStatus();
}
}
}
public class MainCorridor extends Corridor {
public MainCorridor(String id, List<Light> lights, List<AirConditioner> airConditioners) {
super(id, lights, airConditioners);
}
}
public class SubCorridor extends Corridor {
public SubCorridor(String id, List<Light> lights, List<AirConditioner> airConditioners) {
super(id, lights, airConditioners);
}
public List<AirConditioner> getONAirConditioners() {
List<AirConditioner> result = new ArrayList<>();
for (AirConditioner airConditioner : getAirConditioners()) {
if(airConditioner.isON())
result.add(airConditioner);
}
return result;
}
public void motionDetected() {
for (Light light : getLights())
light.switchON();
}
public void clearMotionEvent() {
for (Light light : getLights())
light.switchOFF();
}
}
public class Floor {
private String id;
private Map<String, MainCorridor> mainCorridorMap;
private Map<String, SubCorridor> subCorridorMap;
private Queue<AirConditioner> switchedOFFACs = new LinkedList<>();
public Floor(String id, Map<String, MainCorridor> mainCorridorMap, Map<String, SubCorridor> subCorridorMap) {
this.id = id;
this.mainCorridorMap = mainCorridorMap;
this.subCorridorMap = subCorridorMap;
}
public void motionDetected(String corridorId) {
SubCorridor subCorridor = subCorridorMap.get(corridorId);
subCorridor.motionDetected();
int totalPower = getTotalPower();
int maxPower = getMaxPower();
if (totalPower > maxPower) {
int excessPowerConsumed = totalPower - maxPower;
int numberOfACstoBeSwitchedOFF = (int) Math.round((double) excessPowerConsumed / (double) AirConditioner.SPECPOWER);
switchOFFAC(corridorId, numberOfACstoBeSwitchedOFF);
}
}
public void clearMotionEvent(String corridorId) {
SubCorridor subCorridor = subCorridorMap.get(corridorId);
subCorridor.clearMotionEvent();
int totalPower = getTotalPower();
int maxPower = getMaxPower();
if (totalPower < maxPower) {
int powerAvailableToUse = maxPower - totalPower;
int noOfAcsToTurnOn = (int) Math.floor((double) powerAvailableToUse / (double) AirConditioner.SPECPOWER);
switchONACs(noOfAcsToTurnOn);
}
}
private void switchONACs(int noOfAcsToTurnOn) {
for (int i = 0; i < noOfAcsToTurnOn; i++) {
AirConditioner airConditioner = switchedOFFACs.remove();
airConditioner.switchON();
}
}
private void switchOFFAC(String corridorId, int numberOfACstoBeSwitchedOFF) {
for (String id : subCorridorMap.keySet()) {
if (id.equals(corridorId))
continue;
SubCorridor subCorridor = subCorridorMap.get(id);
List<AirConditioner> onAirConditioners = subCorridor.getONAirConditioners();
for (AirConditioner onAirConditioner : onAirConditioners) {
onAirConditioner.switchOFF();
switchedOFFACs.add(onAirConditioner);
numberOfACstoBeSwitchedOFF--;
}
if (numberOfACstoBeSwitchedOFF == 0)
return;
}
}
private int getTotalPower() {
int result = 0;
for (MainCorridor mainCorridor : mainCorridorMap.values())
result += mainCorridor.getTotalPowerConsumption();
for (SubCorridor subCorridor : subCorridorMap.values())
result += subCorridor.getTotalPowerConsumption();
return result;
}
private int getMaxPower() {
return (mainCorridorMap.size() * 15) + (subCorridorMap.size() * 10);
}
public void printStatus() {
System.out.println(id);
System.out.println();
for (MainCorridor mainCorridor : mainCorridorMap.values()) {
mainCorridor.printStatus();
System.out.println();
}
for (SubCorridor subCorridor : subCorridorMap.values()) {
subCorridor.printStatus();
System.out.println();
}
}
}
public class Hotel {
private String name;
private Map<String,Floor> floorMap;
public Hotel(String name, Map<String, Floor> floorMap) {
this.name = name;
this.floorMap = floorMap;
}
public void cleanFloor(Address address) {
Floor floor = floorMap.get(address.getFloorId());
floor.clearMotionEvent(address.getCorridorId());
}
public void motionDetected(Address address) {
Floor floor = floorMap.get(address.getFloorId());
floor.motionDetected(address.getCorridorId());
}
public void printStatus() {
System.out.println("Power status of Equipments in Hotel " + name);
for (String floorId : floorMap.keySet()) {
Floor floor = floorMap.get(floorId);
floor.printStatus();
}
}
}
The interface for external input which is the motion event in corridors.
public interface Listener {
void eventOccurred(String floorId, String subCorridorId);
}
public class MotionListener implements Listener {
private IController controller;
public MotionListener(IController controller) {
this.controller = controller;
}
@Override
public void eventOccurred(String floorId, String subCorridorId) {
Address address = new Address(floorId, subCorridorId);
controller.actOnMotionEvent(address);
}
}
public class Address {
private String floorId;
private String corridorId;
public Address(String floorId, String corridorId) {
this.floorId = floorId;
this.corridorId = corridorId;
}
public String getFloorId() {
return floorId;
}
public String getCorridorId() {
return corridorId;
}
@Override
public boolean equals(Object o) {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;
Address address = (Address) o;
if (!floorId.equals(address.floorId)) return false;
return corridorId.equals(address.corridorId);
}
@Override
public int hashCode() {
int result = floorId.hashCode();
result = 31 * result + corridorId.hashCode();
return result;
}
}
And the Controller which does the buisiness logic.
public interface IController {
void actOnMotionEvent(Address address);
void destroyMotionWatcher(Address address);
void printStatus();
}
public class Controller implements IController {
private Hotel hotel;
private Map<Address, MotionWatcher> motionWatcherMap = new HashMap<>();
public Controller(int floorCount, int mainCorridorsCount, int subCorridorsCount) {
Map<String, Floor> floorMap = createFloorMap(floorCount, mainCorridorsCount, subCorridorsCount);
this.hotel = new Hotel("MyHotel", floorMap);
}
private Map<String, Floor> createFloorMap(int floorCount, int mainCorridors, int subCorridors) {
Map<String, SubCorridor> subCorridorMap = getSubCorridorMap(subCorridors);
Map<String, MainCorridor> mainCorridorMap = getMainCorridorMap(mainCorridors);
return getFloorMap(floorCount, mainCorridorMap, subCorridorMap);
}
private Map<String, Floor> getFloorMap(int floorCount, Map<String, MainCorridor> mainCorridorMap, Map<String, SubCorridor> subCorridorMap) {
Map<String, Floor> floorMap = new HashMap<>();
for (int i = 0; i < floorCount; i++) {
String id = "Floor-" + i;
Floor floor = new Floor(id, mainCorridorMap, subCorridorMap);
floorMap.put(id, floor);
}
return floorMap;
}
private Map<String, SubCorridor> getSubCorridorMap(int subCorridorsCount) {
int lightId = 0;
int acId = 0;
Map<String, SubCorridor> subCorridorMap = new HashMap<>();
for (int i = 0; i < subCorridorsCount; i++) {
Light light = new Light("Light-" + lightId++);
light.switchOFF();
AirConditioner airConditioner = new AirConditioner("AC-" + acId++);
airConditioner.switchON();
String subCorridorId = "SubCorridor-" + i;
SubCorridor subCorridor = new SubCorridor(subCorridorId, Arrays.asList(light), Arrays.asList(airConditioner));
subCorridorMap.put(subCorridorId, subCorridor);
}
return subCorridorMap;
}
private Map<String, MainCorridor> getMainCorridorMap(int mainCorridorCount) {
int lightId = 0;
int acId = 0;
Map<String, MainCorridor> mainCorridorMap = new HashMap<>();
for (int i = 0; i < mainCorridorCount; i++) {
Light light = new Light("Light-" + lightId++);
light.switchON();
AirConditioner airConditioner = new AirConditioner("AC-" + acId++);
airConditioner.switchON();
String mainCorridorId = "MainCorridor-" + i;
MainCorridor mainCorridor = new MainCorridor(mainCorridorId, Arrays.asList(light), Arrays.asList(airConditioner));
mainCorridorMap.put(mainCorridorId, mainCorridor);
}
return mainCorridorMap;
}
@Override
public void printStatus() {
hotel.printStatus();
}
@Override
public void actOnMotionEvent(Address address) {
hotel.motionDetected(address);
MotionWatcher watcher = motionWatcherMap.get(address);
if (watcher == null) {
watcher = new MotionWatcher(this, address);
motionWatcherMap.put(address, watcher);
new Thread(watcher).start();
return;
}
watcher.renewWaitingPeriod();
}
@Override
public void destroyMotionWatcher(Address address) {
hotel.cleanFloor(address);
motionWatcherMap.remove(address);
System.out.println("After no motion is detected for some specified time ....... ");
hotel.printStatus();
}
}
public class MotionWatcher implements Runnable {
private IController controller;
private Address address;
private static int waitTimeinSeconds = 10;
private volatile boolean gotoSleep = true;
MotionWatcher(IController controller, Address address) {
this.controller = controller;
this.address = address;
}
@Override
public void run() {
long startTime = System.currentTimeMillis();
while (gotoSleep) {
try {
Thread.sleep(waitTimeinSeconds * 1000);
gotoSleep = false;
} catch (InterruptedException e) {
e.printStackTrace();
}
}
System.out.println("Waited for " + (System.currentTimeMillis() - startTime) + " millieseconds.......");
controller.destroyMotionWatcher(address);
}
void renewWaitingPeriod() {
gotoSleep = true;
}
}
For Testing if the code works as expected (in single thread env..)
public class Tester {
public static void main(String args) throws InterruptedException {
IController controller = new Controller(1, 1, 2);
Listener listener = new MotionListener(controller);
System.out.println("INITIAL STAGE...........");
controller.printStatus();
listener.eventOccurred("Floor-0", "SubCorridor-0");
System.out.println("AFTER MOTION DETECTED............");
controller.printStatus();
Thread.sleep(4 * 1000);
listener.eventOccurred("Floor-0", "SubCorridor-0");
System.out.println("AFTER SECOND MOTION DETECTED................");
controller.printStatus();
}
}
Well I do realize there is more than one way to solve a problem using OOP techniques, I just want to know if my OOP solution can be improved (assuming I used and follwed OOP principles). All suggestions or criticism are welcome because the I believe thats the only way to get good on this game.
java object-oriented multithreading simulation
$endgroup$
add a comment |
$begingroup$
A Hotel needs to reduce the overall power used by the equipments installed in its many floors. So the Hotel Management has installed sensors, like Motion Sensors, etc at appropriate places .I have to design a controller which takes inputs from these sensors and controls the various equipments.
- A Hotel can have multiple floors
- Each floor can have multiple main corridors and sub corridors
- Both main corridor and sub corridor have one light each
- Both main and sub corridor lights consume 5 units of power when ON
- Both main and sub corridor have independently controllable ACs
- Both main and sub corridor ACs consume 10 units of power when ON
- All the lights in all the main corridors need to be switched ON
- When a motion is detected in one of the sub corridors the
corresponding lights need to be switched ON - When there is no motion for more than a minute the sub corridor
lights should be switched OFF - The total power consumption of all the ACs and lights combined
should not exceed (Number of Main corridors * 15) + (Number of sub
corridors * 10) units of per floor. Sub corridor AC could be
switched OFF to ensure that the power consumption is not more than
the specified maximum value - When the power consumption goes below the specified maximum value
the ACs that were switched OFF previously must be switched ON
I am writing the Controller program that takes input values for Floors, Main corridors, Sub corridors and takes
different external inputs for motion in sub corridors and for each input prints out the state of all
the lights and ACs in the hotel.
So starting with Modelling the hotel part by parts.
public abstract class Equipment {
private String id;
private boolean isON;
Equipment(String id) {
this.id = id;
}
public boolean isON() {
return isON;
}
public void switchON() {
isON = true;
}
public void switchOFF() {
isON = false;
}
public void printStatus() {
String status = isON() ? "ON" : "OFF";
System.out.println("tt" + id + " " + status);
}
abstract public int getPowerConsumption();
}
public class Light extends Equipment {
public static int SPECPOWER = 5;
public Light(String id) {
super(id);
}
@Override
public int getPowerConsumption() {
return isON() ? 5 : 0;
}
}
public class AirConditioner extends Equipment {
public static int SPECPOWER = 10;
public AirConditioner(String id) {
super(id);
}
@Override
public int getPowerConsumption() {
return isON() ? 10 : 0;
}
}
public abstract class Corridor {
private String id;
private List<Light> lights;
private List<AirConditioner> airConditioners;
public Corridor(String id, List<Light> lights, List<AirConditioner> airConditioners) {
this.id = id;
this.lights = lights;
this.airConditioners = airConditioners;
}
public List<Light> getLights() {
return lights;
}
public List<AirConditioner> getAirConditioners() {
return airConditioners;
}
public int getTotalPowerConsumption() {
int result = 0;
for (Light light : lights)
result += light.getPowerConsumption();
for (AirConditioner airConditioner : airConditioners)
result += airConditioner.getPowerConsumption();
return result;
}
public void printStatus() {
System.out.println("t" + id);
for (Light light : lights) {
light.printStatus();
}
for (AirConditioner airConditioner : airConditioners) {
airConditioner.printStatus();
}
}
}
public class MainCorridor extends Corridor {
public MainCorridor(String id, List<Light> lights, List<AirConditioner> airConditioners) {
super(id, lights, airConditioners);
}
}
public class SubCorridor extends Corridor {
public SubCorridor(String id, List<Light> lights, List<AirConditioner> airConditioners) {
super(id, lights, airConditioners);
}
public List<AirConditioner> getONAirConditioners() {
List<AirConditioner> result = new ArrayList<>();
for (AirConditioner airConditioner : getAirConditioners()) {
if(airConditioner.isON())
result.add(airConditioner);
}
return result;
}
public void motionDetected() {
for (Light light : getLights())
light.switchON();
}
public void clearMotionEvent() {
for (Light light : getLights())
light.switchOFF();
}
}
public class Floor {
private String id;
private Map<String, MainCorridor> mainCorridorMap;
private Map<String, SubCorridor> subCorridorMap;
private Queue<AirConditioner> switchedOFFACs = new LinkedList<>();
public Floor(String id, Map<String, MainCorridor> mainCorridorMap, Map<String, SubCorridor> subCorridorMap) {
this.id = id;
this.mainCorridorMap = mainCorridorMap;
this.subCorridorMap = subCorridorMap;
}
public void motionDetected(String corridorId) {
SubCorridor subCorridor = subCorridorMap.get(corridorId);
subCorridor.motionDetected();
int totalPower = getTotalPower();
int maxPower = getMaxPower();
if (totalPower > maxPower) {
int excessPowerConsumed = totalPower - maxPower;
int numberOfACstoBeSwitchedOFF = (int) Math.round((double) excessPowerConsumed / (double) AirConditioner.SPECPOWER);
switchOFFAC(corridorId, numberOfACstoBeSwitchedOFF);
}
}
public void clearMotionEvent(String corridorId) {
SubCorridor subCorridor = subCorridorMap.get(corridorId);
subCorridor.clearMotionEvent();
int totalPower = getTotalPower();
int maxPower = getMaxPower();
if (totalPower < maxPower) {
int powerAvailableToUse = maxPower - totalPower;
int noOfAcsToTurnOn = (int) Math.floor((double) powerAvailableToUse / (double) AirConditioner.SPECPOWER);
switchONACs(noOfAcsToTurnOn);
}
}
private void switchONACs(int noOfAcsToTurnOn) {
for (int i = 0; i < noOfAcsToTurnOn; i++) {
AirConditioner airConditioner = switchedOFFACs.remove();
airConditioner.switchON();
}
}
private void switchOFFAC(String corridorId, int numberOfACstoBeSwitchedOFF) {
for (String id : subCorridorMap.keySet()) {
if (id.equals(corridorId))
continue;
SubCorridor subCorridor = subCorridorMap.get(id);
List<AirConditioner> onAirConditioners = subCorridor.getONAirConditioners();
for (AirConditioner onAirConditioner : onAirConditioners) {
onAirConditioner.switchOFF();
switchedOFFACs.add(onAirConditioner);
numberOfACstoBeSwitchedOFF--;
}
if (numberOfACstoBeSwitchedOFF == 0)
return;
}
}
private int getTotalPower() {
int result = 0;
for (MainCorridor mainCorridor : mainCorridorMap.values())
result += mainCorridor.getTotalPowerConsumption();
for (SubCorridor subCorridor : subCorridorMap.values())
result += subCorridor.getTotalPowerConsumption();
return result;
}
private int getMaxPower() {
return (mainCorridorMap.size() * 15) + (subCorridorMap.size() * 10);
}
public void printStatus() {
System.out.println(id);
System.out.println();
for (MainCorridor mainCorridor : mainCorridorMap.values()) {
mainCorridor.printStatus();
System.out.println();
}
for (SubCorridor subCorridor : subCorridorMap.values()) {
subCorridor.printStatus();
System.out.println();
}
}
}
public class Hotel {
private String name;
private Map<String,Floor> floorMap;
public Hotel(String name, Map<String, Floor> floorMap) {
this.name = name;
this.floorMap = floorMap;
}
public void cleanFloor(Address address) {
Floor floor = floorMap.get(address.getFloorId());
floor.clearMotionEvent(address.getCorridorId());
}
public void motionDetected(Address address) {
Floor floor = floorMap.get(address.getFloorId());
floor.motionDetected(address.getCorridorId());
}
public void printStatus() {
System.out.println("Power status of Equipments in Hotel " + name);
for (String floorId : floorMap.keySet()) {
Floor floor = floorMap.get(floorId);
floor.printStatus();
}
}
}
The interface for external input which is the motion event in corridors.
public interface Listener {
void eventOccurred(String floorId, String subCorridorId);
}
public class MotionListener implements Listener {
private IController controller;
public MotionListener(IController controller) {
this.controller = controller;
}
@Override
public void eventOccurred(String floorId, String subCorridorId) {
Address address = new Address(floorId, subCorridorId);
controller.actOnMotionEvent(address);
}
}
public class Address {
private String floorId;
private String corridorId;
public Address(String floorId, String corridorId) {
this.floorId = floorId;
this.corridorId = corridorId;
}
public String getFloorId() {
return floorId;
}
public String getCorridorId() {
return corridorId;
}
@Override
public boolean equals(Object o) {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;
Address address = (Address) o;
if (!floorId.equals(address.floorId)) return false;
return corridorId.equals(address.corridorId);
}
@Override
public int hashCode() {
int result = floorId.hashCode();
result = 31 * result + corridorId.hashCode();
return result;
}
}
And the Controller which does the buisiness logic.
public interface IController {
void actOnMotionEvent(Address address);
void destroyMotionWatcher(Address address);
void printStatus();
}
public class Controller implements IController {
private Hotel hotel;
private Map<Address, MotionWatcher> motionWatcherMap = new HashMap<>();
public Controller(int floorCount, int mainCorridorsCount, int subCorridorsCount) {
Map<String, Floor> floorMap = createFloorMap(floorCount, mainCorridorsCount, subCorridorsCount);
this.hotel = new Hotel("MyHotel", floorMap);
}
private Map<String, Floor> createFloorMap(int floorCount, int mainCorridors, int subCorridors) {
Map<String, SubCorridor> subCorridorMap = getSubCorridorMap(subCorridors);
Map<String, MainCorridor> mainCorridorMap = getMainCorridorMap(mainCorridors);
return getFloorMap(floorCount, mainCorridorMap, subCorridorMap);
}
private Map<String, Floor> getFloorMap(int floorCount, Map<String, MainCorridor> mainCorridorMap, Map<String, SubCorridor> subCorridorMap) {
Map<String, Floor> floorMap = new HashMap<>();
for (int i = 0; i < floorCount; i++) {
String id = "Floor-" + i;
Floor floor = new Floor(id, mainCorridorMap, subCorridorMap);
floorMap.put(id, floor);
}
return floorMap;
}
private Map<String, SubCorridor> getSubCorridorMap(int subCorridorsCount) {
int lightId = 0;
int acId = 0;
Map<String, SubCorridor> subCorridorMap = new HashMap<>();
for (int i = 0; i < subCorridorsCount; i++) {
Light light = new Light("Light-" + lightId++);
light.switchOFF();
AirConditioner airConditioner = new AirConditioner("AC-" + acId++);
airConditioner.switchON();
String subCorridorId = "SubCorridor-" + i;
SubCorridor subCorridor = new SubCorridor(subCorridorId, Arrays.asList(light), Arrays.asList(airConditioner));
subCorridorMap.put(subCorridorId, subCorridor);
}
return subCorridorMap;
}
private Map<String, MainCorridor> getMainCorridorMap(int mainCorridorCount) {
int lightId = 0;
int acId = 0;
Map<String, MainCorridor> mainCorridorMap = new HashMap<>();
for (int i = 0; i < mainCorridorCount; i++) {
Light light = new Light("Light-" + lightId++);
light.switchON();
AirConditioner airConditioner = new AirConditioner("AC-" + acId++);
airConditioner.switchON();
String mainCorridorId = "MainCorridor-" + i;
MainCorridor mainCorridor = new MainCorridor(mainCorridorId, Arrays.asList(light), Arrays.asList(airConditioner));
mainCorridorMap.put(mainCorridorId, mainCorridor);
}
return mainCorridorMap;
}
@Override
public void printStatus() {
hotel.printStatus();
}
@Override
public void actOnMotionEvent(Address address) {
hotel.motionDetected(address);
MotionWatcher watcher = motionWatcherMap.get(address);
if (watcher == null) {
watcher = new MotionWatcher(this, address);
motionWatcherMap.put(address, watcher);
new Thread(watcher).start();
return;
}
watcher.renewWaitingPeriod();
}
@Override
public void destroyMotionWatcher(Address address) {
hotel.cleanFloor(address);
motionWatcherMap.remove(address);
System.out.println("After no motion is detected for some specified time ....... ");
hotel.printStatus();
}
}
public class MotionWatcher implements Runnable {
private IController controller;
private Address address;
private static int waitTimeinSeconds = 10;
private volatile boolean gotoSleep = true;
MotionWatcher(IController controller, Address address) {
this.controller = controller;
this.address = address;
}
@Override
public void run() {
long startTime = System.currentTimeMillis();
while (gotoSleep) {
try {
Thread.sleep(waitTimeinSeconds * 1000);
gotoSleep = false;
} catch (InterruptedException e) {
e.printStackTrace();
}
}
System.out.println("Waited for " + (System.currentTimeMillis() - startTime) + " millieseconds.......");
controller.destroyMotionWatcher(address);
}
void renewWaitingPeriod() {
gotoSleep = true;
}
}
For Testing if the code works as expected (in single thread env..)
public class Tester {
public static void main(String args) throws InterruptedException {
IController controller = new Controller(1, 1, 2);
Listener listener = new MotionListener(controller);
System.out.println("INITIAL STAGE...........");
controller.printStatus();
listener.eventOccurred("Floor-0", "SubCorridor-0");
System.out.println("AFTER MOTION DETECTED............");
controller.printStatus();
Thread.sleep(4 * 1000);
listener.eventOccurred("Floor-0", "SubCorridor-0");
System.out.println("AFTER SECOND MOTION DETECTED................");
controller.printStatus();
}
}
Well I do realize there is more than one way to solve a problem using OOP techniques, I just want to know if my OOP solution can be improved (assuming I used and follwed OOP principles). All suggestions or criticism are welcome because the I believe thats the only way to get good on this game.
java object-oriented multithreading simulation
$endgroup$
add a comment |
$begingroup$
A Hotel needs to reduce the overall power used by the equipments installed in its many floors. So the Hotel Management has installed sensors, like Motion Sensors, etc at appropriate places .I have to design a controller which takes inputs from these sensors and controls the various equipments.
- A Hotel can have multiple floors
- Each floor can have multiple main corridors and sub corridors
- Both main corridor and sub corridor have one light each
- Both main and sub corridor lights consume 5 units of power when ON
- Both main and sub corridor have independently controllable ACs
- Both main and sub corridor ACs consume 10 units of power when ON
- All the lights in all the main corridors need to be switched ON
- When a motion is detected in one of the sub corridors the
corresponding lights need to be switched ON - When there is no motion for more than a minute the sub corridor
lights should be switched OFF - The total power consumption of all the ACs and lights combined
should not exceed (Number of Main corridors * 15) + (Number of sub
corridors * 10) units of per floor. Sub corridor AC could be
switched OFF to ensure that the power consumption is not more than
the specified maximum value - When the power consumption goes below the specified maximum value
the ACs that were switched OFF previously must be switched ON
I am writing the Controller program that takes input values for Floors, Main corridors, Sub corridors and takes
different external inputs for motion in sub corridors and for each input prints out the state of all
the lights and ACs in the hotel.
So starting with Modelling the hotel part by parts.
public abstract class Equipment {
private String id;
private boolean isON;
Equipment(String id) {
this.id = id;
}
public boolean isON() {
return isON;
}
public void switchON() {
isON = true;
}
public void switchOFF() {
isON = false;
}
public void printStatus() {
String status = isON() ? "ON" : "OFF";
System.out.println("tt" + id + " " + status);
}
abstract public int getPowerConsumption();
}
public class Light extends Equipment {
public static int SPECPOWER = 5;
public Light(String id) {
super(id);
}
@Override
public int getPowerConsumption() {
return isON() ? 5 : 0;
}
}
public class AirConditioner extends Equipment {
public static int SPECPOWER = 10;
public AirConditioner(String id) {
super(id);
}
@Override
public int getPowerConsumption() {
return isON() ? 10 : 0;
}
}
public abstract class Corridor {
private String id;
private List<Light> lights;
private List<AirConditioner> airConditioners;
public Corridor(String id, List<Light> lights, List<AirConditioner> airConditioners) {
this.id = id;
this.lights = lights;
this.airConditioners = airConditioners;
}
public List<Light> getLights() {
return lights;
}
public List<AirConditioner> getAirConditioners() {
return airConditioners;
}
public int getTotalPowerConsumption() {
int result = 0;
for (Light light : lights)
result += light.getPowerConsumption();
for (AirConditioner airConditioner : airConditioners)
result += airConditioner.getPowerConsumption();
return result;
}
public void printStatus() {
System.out.println("t" + id);
for (Light light : lights) {
light.printStatus();
}
for (AirConditioner airConditioner : airConditioners) {
airConditioner.printStatus();
}
}
}
public class MainCorridor extends Corridor {
public MainCorridor(String id, List<Light> lights, List<AirConditioner> airConditioners) {
super(id, lights, airConditioners);
}
}
public class SubCorridor extends Corridor {
public SubCorridor(String id, List<Light> lights, List<AirConditioner> airConditioners) {
super(id, lights, airConditioners);
}
public List<AirConditioner> getONAirConditioners() {
List<AirConditioner> result = new ArrayList<>();
for (AirConditioner airConditioner : getAirConditioners()) {
if(airConditioner.isON())
result.add(airConditioner);
}
return result;
}
public void motionDetected() {
for (Light light : getLights())
light.switchON();
}
public void clearMotionEvent() {
for (Light light : getLights())
light.switchOFF();
}
}
public class Floor {
private String id;
private Map<String, MainCorridor> mainCorridorMap;
private Map<String, SubCorridor> subCorridorMap;
private Queue<AirConditioner> switchedOFFACs = new LinkedList<>();
public Floor(String id, Map<String, MainCorridor> mainCorridorMap, Map<String, SubCorridor> subCorridorMap) {
this.id = id;
this.mainCorridorMap = mainCorridorMap;
this.subCorridorMap = subCorridorMap;
}
public void motionDetected(String corridorId) {
SubCorridor subCorridor = subCorridorMap.get(corridorId);
subCorridor.motionDetected();
int totalPower = getTotalPower();
int maxPower = getMaxPower();
if (totalPower > maxPower) {
int excessPowerConsumed = totalPower - maxPower;
int numberOfACstoBeSwitchedOFF = (int) Math.round((double) excessPowerConsumed / (double) AirConditioner.SPECPOWER);
switchOFFAC(corridorId, numberOfACstoBeSwitchedOFF);
}
}
public void clearMotionEvent(String corridorId) {
SubCorridor subCorridor = subCorridorMap.get(corridorId);
subCorridor.clearMotionEvent();
int totalPower = getTotalPower();
int maxPower = getMaxPower();
if (totalPower < maxPower) {
int powerAvailableToUse = maxPower - totalPower;
int noOfAcsToTurnOn = (int) Math.floor((double) powerAvailableToUse / (double) AirConditioner.SPECPOWER);
switchONACs(noOfAcsToTurnOn);
}
}
private void switchONACs(int noOfAcsToTurnOn) {
for (int i = 0; i < noOfAcsToTurnOn; i++) {
AirConditioner airConditioner = switchedOFFACs.remove();
airConditioner.switchON();
}
}
private void switchOFFAC(String corridorId, int numberOfACstoBeSwitchedOFF) {
for (String id : subCorridorMap.keySet()) {
if (id.equals(corridorId))
continue;
SubCorridor subCorridor = subCorridorMap.get(id);
List<AirConditioner> onAirConditioners = subCorridor.getONAirConditioners();
for (AirConditioner onAirConditioner : onAirConditioners) {
onAirConditioner.switchOFF();
switchedOFFACs.add(onAirConditioner);
numberOfACstoBeSwitchedOFF--;
}
if (numberOfACstoBeSwitchedOFF == 0)
return;
}
}
private int getTotalPower() {
int result = 0;
for (MainCorridor mainCorridor : mainCorridorMap.values())
result += mainCorridor.getTotalPowerConsumption();
for (SubCorridor subCorridor : subCorridorMap.values())
result += subCorridor.getTotalPowerConsumption();
return result;
}
private int getMaxPower() {
return (mainCorridorMap.size() * 15) + (subCorridorMap.size() * 10);
}
public void printStatus() {
System.out.println(id);
System.out.println();
for (MainCorridor mainCorridor : mainCorridorMap.values()) {
mainCorridor.printStatus();
System.out.println();
}
for (SubCorridor subCorridor : subCorridorMap.values()) {
subCorridor.printStatus();
System.out.println();
}
}
}
public class Hotel {
private String name;
private Map<String,Floor> floorMap;
public Hotel(String name, Map<String, Floor> floorMap) {
this.name = name;
this.floorMap = floorMap;
}
public void cleanFloor(Address address) {
Floor floor = floorMap.get(address.getFloorId());
floor.clearMotionEvent(address.getCorridorId());
}
public void motionDetected(Address address) {
Floor floor = floorMap.get(address.getFloorId());
floor.motionDetected(address.getCorridorId());
}
public void printStatus() {
System.out.println("Power status of Equipments in Hotel " + name);
for (String floorId : floorMap.keySet()) {
Floor floor = floorMap.get(floorId);
floor.printStatus();
}
}
}
The interface for external input which is the motion event in corridors.
public interface Listener {
void eventOccurred(String floorId, String subCorridorId);
}
public class MotionListener implements Listener {
private IController controller;
public MotionListener(IController controller) {
this.controller = controller;
}
@Override
public void eventOccurred(String floorId, String subCorridorId) {
Address address = new Address(floorId, subCorridorId);
controller.actOnMotionEvent(address);
}
}
public class Address {
private String floorId;
private String corridorId;
public Address(String floorId, String corridorId) {
this.floorId = floorId;
this.corridorId = corridorId;
}
public String getFloorId() {
return floorId;
}
public String getCorridorId() {
return corridorId;
}
@Override
public boolean equals(Object o) {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;
Address address = (Address) o;
if (!floorId.equals(address.floorId)) return false;
return corridorId.equals(address.corridorId);
}
@Override
public int hashCode() {
int result = floorId.hashCode();
result = 31 * result + corridorId.hashCode();
return result;
}
}
And the Controller which does the buisiness logic.
public interface IController {
void actOnMotionEvent(Address address);
void destroyMotionWatcher(Address address);
void printStatus();
}
public class Controller implements IController {
private Hotel hotel;
private Map<Address, MotionWatcher> motionWatcherMap = new HashMap<>();
public Controller(int floorCount, int mainCorridorsCount, int subCorridorsCount) {
Map<String, Floor> floorMap = createFloorMap(floorCount, mainCorridorsCount, subCorridorsCount);
this.hotel = new Hotel("MyHotel", floorMap);
}
private Map<String, Floor> createFloorMap(int floorCount, int mainCorridors, int subCorridors) {
Map<String, SubCorridor> subCorridorMap = getSubCorridorMap(subCorridors);
Map<String, MainCorridor> mainCorridorMap = getMainCorridorMap(mainCorridors);
return getFloorMap(floorCount, mainCorridorMap, subCorridorMap);
}
private Map<String, Floor> getFloorMap(int floorCount, Map<String, MainCorridor> mainCorridorMap, Map<String, SubCorridor> subCorridorMap) {
Map<String, Floor> floorMap = new HashMap<>();
for (int i = 0; i < floorCount; i++) {
String id = "Floor-" + i;
Floor floor = new Floor(id, mainCorridorMap, subCorridorMap);
floorMap.put(id, floor);
}
return floorMap;
}
private Map<String, SubCorridor> getSubCorridorMap(int subCorridorsCount) {
int lightId = 0;
int acId = 0;
Map<String, SubCorridor> subCorridorMap = new HashMap<>();
for (int i = 0; i < subCorridorsCount; i++) {
Light light = new Light("Light-" + lightId++);
light.switchOFF();
AirConditioner airConditioner = new AirConditioner("AC-" + acId++);
airConditioner.switchON();
String subCorridorId = "SubCorridor-" + i;
SubCorridor subCorridor = new SubCorridor(subCorridorId, Arrays.asList(light), Arrays.asList(airConditioner));
subCorridorMap.put(subCorridorId, subCorridor);
}
return subCorridorMap;
}
private Map<String, MainCorridor> getMainCorridorMap(int mainCorridorCount) {
int lightId = 0;
int acId = 0;
Map<String, MainCorridor> mainCorridorMap = new HashMap<>();
for (int i = 0; i < mainCorridorCount; i++) {
Light light = new Light("Light-" + lightId++);
light.switchON();
AirConditioner airConditioner = new AirConditioner("AC-" + acId++);
airConditioner.switchON();
String mainCorridorId = "MainCorridor-" + i;
MainCorridor mainCorridor = new MainCorridor(mainCorridorId, Arrays.asList(light), Arrays.asList(airConditioner));
mainCorridorMap.put(mainCorridorId, mainCorridor);
}
return mainCorridorMap;
}
@Override
public void printStatus() {
hotel.printStatus();
}
@Override
public void actOnMotionEvent(Address address) {
hotel.motionDetected(address);
MotionWatcher watcher = motionWatcherMap.get(address);
if (watcher == null) {
watcher = new MotionWatcher(this, address);
motionWatcherMap.put(address, watcher);
new Thread(watcher).start();
return;
}
watcher.renewWaitingPeriod();
}
@Override
public void destroyMotionWatcher(Address address) {
hotel.cleanFloor(address);
motionWatcherMap.remove(address);
System.out.println("After no motion is detected for some specified time ....... ");
hotel.printStatus();
}
}
public class MotionWatcher implements Runnable {
private IController controller;
private Address address;
private static int waitTimeinSeconds = 10;
private volatile boolean gotoSleep = true;
MotionWatcher(IController controller, Address address) {
this.controller = controller;
this.address = address;
}
@Override
public void run() {
long startTime = System.currentTimeMillis();
while (gotoSleep) {
try {
Thread.sleep(waitTimeinSeconds * 1000);
gotoSleep = false;
} catch (InterruptedException e) {
e.printStackTrace();
}
}
System.out.println("Waited for " + (System.currentTimeMillis() - startTime) + " millieseconds.......");
controller.destroyMotionWatcher(address);
}
void renewWaitingPeriod() {
gotoSleep = true;
}
}
For Testing if the code works as expected (in single thread env..)
public class Tester {
public static void main(String args) throws InterruptedException {
IController controller = new Controller(1, 1, 2);
Listener listener = new MotionListener(controller);
System.out.println("INITIAL STAGE...........");
controller.printStatus();
listener.eventOccurred("Floor-0", "SubCorridor-0");
System.out.println("AFTER MOTION DETECTED............");
controller.printStatus();
Thread.sleep(4 * 1000);
listener.eventOccurred("Floor-0", "SubCorridor-0");
System.out.println("AFTER SECOND MOTION DETECTED................");
controller.printStatus();
}
}
Well I do realize there is more than one way to solve a problem using OOP techniques, I just want to know if my OOP solution can be improved (assuming I used and follwed OOP principles). All suggestions or criticism are welcome because the I believe thats the only way to get good on this game.
java object-oriented multithreading simulation
$endgroup$
A Hotel needs to reduce the overall power used by the equipments installed in its many floors. So the Hotel Management has installed sensors, like Motion Sensors, etc at appropriate places .I have to design a controller which takes inputs from these sensors and controls the various equipments.
- A Hotel can have multiple floors
- Each floor can have multiple main corridors and sub corridors
- Both main corridor and sub corridor have one light each
- Both main and sub corridor lights consume 5 units of power when ON
- Both main and sub corridor have independently controllable ACs
- Both main and sub corridor ACs consume 10 units of power when ON
- All the lights in all the main corridors need to be switched ON
- When a motion is detected in one of the sub corridors the
corresponding lights need to be switched ON - When there is no motion for more than a minute the sub corridor
lights should be switched OFF - The total power consumption of all the ACs and lights combined
should not exceed (Number of Main corridors * 15) + (Number of sub
corridors * 10) units of per floor. Sub corridor AC could be
switched OFF to ensure that the power consumption is not more than
the specified maximum value - When the power consumption goes below the specified maximum value
the ACs that were switched OFF previously must be switched ON
I am writing the Controller program that takes input values for Floors, Main corridors, Sub corridors and takes
different external inputs for motion in sub corridors and for each input prints out the state of all
the lights and ACs in the hotel.
So starting with Modelling the hotel part by parts.
public abstract class Equipment {
private String id;
private boolean isON;
Equipment(String id) {
this.id = id;
}
public boolean isON() {
return isON;
}
public void switchON() {
isON = true;
}
public void switchOFF() {
isON = false;
}
public void printStatus() {
String status = isON() ? "ON" : "OFF";
System.out.println("tt" + id + " " + status);
}
abstract public int getPowerConsumption();
}
public class Light extends Equipment {
public static int SPECPOWER = 5;
public Light(String id) {
super(id);
}
@Override
public int getPowerConsumption() {
return isON() ? 5 : 0;
}
}
public class AirConditioner extends Equipment {
public static int SPECPOWER = 10;
public AirConditioner(String id) {
super(id);
}
@Override
public int getPowerConsumption() {
return isON() ? 10 : 0;
}
}
public abstract class Corridor {
private String id;
private List<Light> lights;
private List<AirConditioner> airConditioners;
public Corridor(String id, List<Light> lights, List<AirConditioner> airConditioners) {
this.id = id;
this.lights = lights;
this.airConditioners = airConditioners;
}
public List<Light> getLights() {
return lights;
}
public List<AirConditioner> getAirConditioners() {
return airConditioners;
}
public int getTotalPowerConsumption() {
int result = 0;
for (Light light : lights)
result += light.getPowerConsumption();
for (AirConditioner airConditioner : airConditioners)
result += airConditioner.getPowerConsumption();
return result;
}
public void printStatus() {
System.out.println("t" + id);
for (Light light : lights) {
light.printStatus();
}
for (AirConditioner airConditioner : airConditioners) {
airConditioner.printStatus();
}
}
}
public class MainCorridor extends Corridor {
public MainCorridor(String id, List<Light> lights, List<AirConditioner> airConditioners) {
super(id, lights, airConditioners);
}
}
public class SubCorridor extends Corridor {
public SubCorridor(String id, List<Light> lights, List<AirConditioner> airConditioners) {
super(id, lights, airConditioners);
}
public List<AirConditioner> getONAirConditioners() {
List<AirConditioner> result = new ArrayList<>();
for (AirConditioner airConditioner : getAirConditioners()) {
if(airConditioner.isON())
result.add(airConditioner);
}
return result;
}
public void motionDetected() {
for (Light light : getLights())
light.switchON();
}
public void clearMotionEvent() {
for (Light light : getLights())
light.switchOFF();
}
}
public class Floor {
private String id;
private Map<String, MainCorridor> mainCorridorMap;
private Map<String, SubCorridor> subCorridorMap;
private Queue<AirConditioner> switchedOFFACs = new LinkedList<>();
public Floor(String id, Map<String, MainCorridor> mainCorridorMap, Map<String, SubCorridor> subCorridorMap) {
this.id = id;
this.mainCorridorMap = mainCorridorMap;
this.subCorridorMap = subCorridorMap;
}
public void motionDetected(String corridorId) {
SubCorridor subCorridor = subCorridorMap.get(corridorId);
subCorridor.motionDetected();
int totalPower = getTotalPower();
int maxPower = getMaxPower();
if (totalPower > maxPower) {
int excessPowerConsumed = totalPower - maxPower;
int numberOfACstoBeSwitchedOFF = (int) Math.round((double) excessPowerConsumed / (double) AirConditioner.SPECPOWER);
switchOFFAC(corridorId, numberOfACstoBeSwitchedOFF);
}
}
public void clearMotionEvent(String corridorId) {
SubCorridor subCorridor = subCorridorMap.get(corridorId);
subCorridor.clearMotionEvent();
int totalPower = getTotalPower();
int maxPower = getMaxPower();
if (totalPower < maxPower) {
int powerAvailableToUse = maxPower - totalPower;
int noOfAcsToTurnOn = (int) Math.floor((double) powerAvailableToUse / (double) AirConditioner.SPECPOWER);
switchONACs(noOfAcsToTurnOn);
}
}
private void switchONACs(int noOfAcsToTurnOn) {
for (int i = 0; i < noOfAcsToTurnOn; i++) {
AirConditioner airConditioner = switchedOFFACs.remove();
airConditioner.switchON();
}
}
private void switchOFFAC(String corridorId, int numberOfACstoBeSwitchedOFF) {
for (String id : subCorridorMap.keySet()) {
if (id.equals(corridorId))
continue;
SubCorridor subCorridor = subCorridorMap.get(id);
List<AirConditioner> onAirConditioners = subCorridor.getONAirConditioners();
for (AirConditioner onAirConditioner : onAirConditioners) {
onAirConditioner.switchOFF();
switchedOFFACs.add(onAirConditioner);
numberOfACstoBeSwitchedOFF--;
}
if (numberOfACstoBeSwitchedOFF == 0)
return;
}
}
private int getTotalPower() {
int result = 0;
for (MainCorridor mainCorridor : mainCorridorMap.values())
result += mainCorridor.getTotalPowerConsumption();
for (SubCorridor subCorridor : subCorridorMap.values())
result += subCorridor.getTotalPowerConsumption();
return result;
}
private int getMaxPower() {
return (mainCorridorMap.size() * 15) + (subCorridorMap.size() * 10);
}
public void printStatus() {
System.out.println(id);
System.out.println();
for (MainCorridor mainCorridor : mainCorridorMap.values()) {
mainCorridor.printStatus();
System.out.println();
}
for (SubCorridor subCorridor : subCorridorMap.values()) {
subCorridor.printStatus();
System.out.println();
}
}
}
public class Hotel {
private String name;
private Map<String,Floor> floorMap;
public Hotel(String name, Map<String, Floor> floorMap) {
this.name = name;
this.floorMap = floorMap;
}
public void cleanFloor(Address address) {
Floor floor = floorMap.get(address.getFloorId());
floor.clearMotionEvent(address.getCorridorId());
}
public void motionDetected(Address address) {
Floor floor = floorMap.get(address.getFloorId());
floor.motionDetected(address.getCorridorId());
}
public void printStatus() {
System.out.println("Power status of Equipments in Hotel " + name);
for (String floorId : floorMap.keySet()) {
Floor floor = floorMap.get(floorId);
floor.printStatus();
}
}
}
The interface for external input which is the motion event in corridors.
public interface Listener {
void eventOccurred(String floorId, String subCorridorId);
}
public class MotionListener implements Listener {
private IController controller;
public MotionListener(IController controller) {
this.controller = controller;
}
@Override
public void eventOccurred(String floorId, String subCorridorId) {
Address address = new Address(floorId, subCorridorId);
controller.actOnMotionEvent(address);
}
}
public class Address {
private String floorId;
private String corridorId;
public Address(String floorId, String corridorId) {
this.floorId = floorId;
this.corridorId = corridorId;
}
public String getFloorId() {
return floorId;
}
public String getCorridorId() {
return corridorId;
}
@Override
public boolean equals(Object o) {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;
Address address = (Address) o;
if (!floorId.equals(address.floorId)) return false;
return corridorId.equals(address.corridorId);
}
@Override
public int hashCode() {
int result = floorId.hashCode();
result = 31 * result + corridorId.hashCode();
return result;
}
}
And the Controller which does the buisiness logic.
public interface IController {
void actOnMotionEvent(Address address);
void destroyMotionWatcher(Address address);
void printStatus();
}
public class Controller implements IController {
private Hotel hotel;
private Map<Address, MotionWatcher> motionWatcherMap = new HashMap<>();
public Controller(int floorCount, int mainCorridorsCount, int subCorridorsCount) {
Map<String, Floor> floorMap = createFloorMap(floorCount, mainCorridorsCount, subCorridorsCount);
this.hotel = new Hotel("MyHotel", floorMap);
}
private Map<String, Floor> createFloorMap(int floorCount, int mainCorridors, int subCorridors) {
Map<String, SubCorridor> subCorridorMap = getSubCorridorMap(subCorridors);
Map<String, MainCorridor> mainCorridorMap = getMainCorridorMap(mainCorridors);
return getFloorMap(floorCount, mainCorridorMap, subCorridorMap);
}
private Map<String, Floor> getFloorMap(int floorCount, Map<String, MainCorridor> mainCorridorMap, Map<String, SubCorridor> subCorridorMap) {
Map<String, Floor> floorMap = new HashMap<>();
for (int i = 0; i < floorCount; i++) {
String id = "Floor-" + i;
Floor floor = new Floor(id, mainCorridorMap, subCorridorMap);
floorMap.put(id, floor);
}
return floorMap;
}
private Map<String, SubCorridor> getSubCorridorMap(int subCorridorsCount) {
int lightId = 0;
int acId = 0;
Map<String, SubCorridor> subCorridorMap = new HashMap<>();
for (int i = 0; i < subCorridorsCount; i++) {
Light light = new Light("Light-" + lightId++);
light.switchOFF();
AirConditioner airConditioner = new AirConditioner("AC-" + acId++);
airConditioner.switchON();
String subCorridorId = "SubCorridor-" + i;
SubCorridor subCorridor = new SubCorridor(subCorridorId, Arrays.asList(light), Arrays.asList(airConditioner));
subCorridorMap.put(subCorridorId, subCorridor);
}
return subCorridorMap;
}
private Map<String, MainCorridor> getMainCorridorMap(int mainCorridorCount) {
int lightId = 0;
int acId = 0;
Map<String, MainCorridor> mainCorridorMap = new HashMap<>();
for (int i = 0; i < mainCorridorCount; i++) {
Light light = new Light("Light-" + lightId++);
light.switchON();
AirConditioner airConditioner = new AirConditioner("AC-" + acId++);
airConditioner.switchON();
String mainCorridorId = "MainCorridor-" + i;
MainCorridor mainCorridor = new MainCorridor(mainCorridorId, Arrays.asList(light), Arrays.asList(airConditioner));
mainCorridorMap.put(mainCorridorId, mainCorridor);
}
return mainCorridorMap;
}
@Override
public void printStatus() {
hotel.printStatus();
}
@Override
public void actOnMotionEvent(Address address) {
hotel.motionDetected(address);
MotionWatcher watcher = motionWatcherMap.get(address);
if (watcher == null) {
watcher = new MotionWatcher(this, address);
motionWatcherMap.put(address, watcher);
new Thread(watcher).start();
return;
}
watcher.renewWaitingPeriod();
}
@Override
public void destroyMotionWatcher(Address address) {
hotel.cleanFloor(address);
motionWatcherMap.remove(address);
System.out.println("After no motion is detected for some specified time ....... ");
hotel.printStatus();
}
}
public class MotionWatcher implements Runnable {
private IController controller;
private Address address;
private static int waitTimeinSeconds = 10;
private volatile boolean gotoSleep = true;
MotionWatcher(IController controller, Address address) {
this.controller = controller;
this.address = address;
}
@Override
public void run() {
long startTime = System.currentTimeMillis();
while (gotoSleep) {
try {
Thread.sleep(waitTimeinSeconds * 1000);
gotoSleep = false;
} catch (InterruptedException e) {
e.printStackTrace();
}
}
System.out.println("Waited for " + (System.currentTimeMillis() - startTime) + " millieseconds.......");
controller.destroyMotionWatcher(address);
}
void renewWaitingPeriod() {
gotoSleep = true;
}
}
For Testing if the code works as expected (in single thread env..)
public class Tester {
public static void main(String args) throws InterruptedException {
IController controller = new Controller(1, 1, 2);
Listener listener = new MotionListener(controller);
System.out.println("INITIAL STAGE...........");
controller.printStatus();
listener.eventOccurred("Floor-0", "SubCorridor-0");
System.out.println("AFTER MOTION DETECTED............");
controller.printStatus();
Thread.sleep(4 * 1000);
listener.eventOccurred("Floor-0", "SubCorridor-0");
System.out.println("AFTER SECOND MOTION DETECTED................");
controller.printStatus();
}
}
Well I do realize there is more than one way to solve a problem using OOP techniques, I just want to know if my OOP solution can be improved (assuming I used and follwed OOP principles). All suggestions or criticism are welcome because the I believe thats the only way to get good on this game.
java object-oriented multithreading simulation
java object-oriented multithreading simulation
edited Oct 19 '17 at 12:20
200_success
130k17155419
130k17155419
asked Oct 19 '17 at 4:07
Tiny RickTiny Rick
2161310
2161310
add a comment |
add a comment |
3 Answers
3
active
oldest
votes
$begingroup$
Not a full review, but some initial thoughts:
Structure
- You define
SPECPOWER
, but still hardcode the value inside methods. This will surely lead to bugs if you decide to change only the field. - Is there equipment that can be on or off, but which doesn't consume power? If not, I would pull power into equipment (and rename it). If yes, I would add a generic electronic device class. Either will get rid of the duplicate
getPowerConsumption
implementation. - I don't like that
getTotalPowerConsumption
and similar methods need to know about the specific devices (lights, airconditioners). If you need to add say a security camera later on, you will need to add it in many places, and forgetting some will lead to bugs. Possible solutions would be to put all devices in a devices list (which might cause performance issues if you only want to get one specific type), or a different structure such as a map. Your constructors would also need to change then. - Why does a floor have
switchedOFFACs
when the corridors already have the ACs? This doesn't seem necessary and is confusing. Also, is there a reason that it is a queue?
Documentation
You don't have any. I would think about adding javaDoc comments to at least some methods. Eg what does getMaxPower
do? And how is it different from getTotalPower
? You probably know now, but will you remember in a couple of months?
Naming
- Try to be consistent with your names, eg:
AC
vsAirconditioner
- I wouldn't use reverse hungarian notation (but if you do, it should be consistent);
mainCorridorMap
should simply bemainCorridors
. This is especially confusing as there are things called floor maps, but yourfloorMap
field doesn't contain one.
Address
seems like a confusing name. I don't think that you need it at all, but if you do, maybeCorridorLocation
?
clearMotionEvent
: Here, we aren't really dealing with any events. I would probably rename it toturnLightsOff
, andmotionDetected
toturnLightsOn
.
Misc
- Always use curly brackets, even for one-line statements. Not using them can cause bugs. (but if you disagree, you should be consistent)
$endgroup$
$begingroup$
thanks for the review. Answering for "Why does a floor have switchedOFFACs when the corridors already have the ACs? This doesn't seem necessary and is confusing. Also, is there a reason that it is a queue? " Because only the Floor has access to ACs in different corridors. The reason why I kept it in a Queue is, due to some motion event, we may have switched AC1 first. Then comes another event and we are switching off another AC and so on. When we have adequete power at some point, it has been so long since I turned off AC1. So in order to be fair, I put those AC's in queue.
$endgroup$
– Tiny Rick
Oct 19 '17 at 8:17
$begingroup$
Regarding Point number 2 in "Structure".... Great point by the way :) I guess I shoud have a dedicated Collection for Electronic Equipments which can include Lights, ACs, and Security Camera...etc. That collections gets updated during the Corridor object constructor. Then all the powerconsumption computing methods can just refer that collection.
$endgroup$
– Tiny Rick
Oct 19 '17 at 8:27
add a comment |
$begingroup$
Thanks for sharing your code.
OOP
OOP doesn't mean to "split up" code into random classes.
The ultimate goal of OOP is to reduce code duplication, improve readability and support reuse as well as extending the code.
Doing OOP means that you follow certain principles which are (among others):
- information hiding / encapsulation
- single responsibility
- separation of concerns
- KISS (Keep it simple (and) stupid.)
- DRY (Don't repeat yourself.)
- "Tell! Don't ask."
- Law of demeter ("Don't talk to strangers!")
One basic rule of thumb is: create a new class if there is a difference in behavior. your classes Light
and AirConditioner
only differ in configuration (the number they return for power consumption when on).
Naming
Finding good names is the hardest part in programming, so always take your time to think about the names of your identifiers.
Naming Conventions
Please read (and follow) the
Java Naming Conventions
E.g.: your methods and variables have names like switchOFF
and isON
which should better be switchOff
and isOn
.
misleading naming
Some of your methods have somewhat misleading names.
E.g.: getMaxPower()
implies the access to an objects property while it in deed calculates the max power. Therefore it should better be named calculatePowerConsuption()
.
$endgroup$
$begingroup$
Thanks Timothy for taking you valuable time to review my code. While I agree on your suggestion on the Naming conventions, I am not quite convinced why Light and AirConditioner should be modelled by a single class just varying by the power meter parameter. The reason I can say is, we have to switch on lights in case of a motion event and switch off ACs. If I model them by a single class I need to get and check the power meter every time. IMHO, I find it more comfortable and readable if we have seperate classes for them.
$endgroup$
– Tiny Rick
Oct 19 '17 at 9:19
1
$begingroup$
@TinyRick "we have to switch on lights in case of a motion event and switch off ACs." Your current implementation does not have this different behavior in theLight
andAirConditioner
classes. As mentioned this classes only differ in a constant value. Of cause you might move this behavior there but with your current design these two classes really are one.
$endgroup$
– Timothy Truckle
Oct 19 '17 at 10:42
add a comment |
$begingroup$
how to execute this program on eclipse?
New contributor
$endgroup$
add a comment |
Your Answer
StackExchange.ifUsing("editor", function () {
return StackExchange.using("mathjaxEditing", function () {
StackExchange.MarkdownEditor.creationCallbacks.add(function (editor, postfix) {
StackExchange.mathjaxEditing.prepareWmdForMathJax(editor, postfix, [["\$", "\$"]]);
});
});
}, "mathjax-editing");
StackExchange.ifUsing("editor", function () {
StackExchange.using("externalEditor", function () {
StackExchange.using("snippets", function () {
StackExchange.snippets.init();
});
});
}, "code-snippets");
StackExchange.ready(function() {
var channelOptions = {
tags: "".split(" "),
id: "196"
};
initTagRenderer("".split(" "), "".split(" "), channelOptions);
StackExchange.using("externalEditor", function() {
// Have to fire editor after snippets, if snippets enabled
if (StackExchange.settings.snippets.snippetsEnabled) {
StackExchange.using("snippets", function() {
createEditor();
});
}
else {
createEditor();
}
});
function createEditor() {
StackExchange.prepareEditor({
heartbeatType: 'answer',
autoActivateHeartbeat: false,
convertImagesToLinks: false,
noModals: true,
showLowRepImageUploadWarning: true,
reputationToPostImages: null,
bindNavPrevention: true,
postfix: "",
imageUploader: {
brandingHtml: "Powered by u003ca class="icon-imgur-white" href="https://imgur.com/"u003eu003c/au003e",
contentPolicyHtml: "User contributions licensed under u003ca href="https://creativecommons.org/licenses/by-sa/3.0/"u003ecc by-sa 3.0 with attribution requiredu003c/au003e u003ca href="https://stackoverflow.com/legal/content-policy"u003e(content policy)u003c/au003e",
allowUrls: true
},
onDemand: true,
discardSelector: ".discard-answer"
,immediatelyShowMarkdownHelp:true
});
}
});
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
StackExchange.ready(
function () {
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f178255%2fhotel-power-management%23new-answer', 'question_page');
}
);
Post as a guest
Required, but never shown
3 Answers
3
active
oldest
votes
3 Answers
3
active
oldest
votes
active
oldest
votes
active
oldest
votes
$begingroup$
Not a full review, but some initial thoughts:
Structure
- You define
SPECPOWER
, but still hardcode the value inside methods. This will surely lead to bugs if you decide to change only the field. - Is there equipment that can be on or off, but which doesn't consume power? If not, I would pull power into equipment (and rename it). If yes, I would add a generic electronic device class. Either will get rid of the duplicate
getPowerConsumption
implementation. - I don't like that
getTotalPowerConsumption
and similar methods need to know about the specific devices (lights, airconditioners). If you need to add say a security camera later on, you will need to add it in many places, and forgetting some will lead to bugs. Possible solutions would be to put all devices in a devices list (which might cause performance issues if you only want to get one specific type), or a different structure such as a map. Your constructors would also need to change then. - Why does a floor have
switchedOFFACs
when the corridors already have the ACs? This doesn't seem necessary and is confusing. Also, is there a reason that it is a queue?
Documentation
You don't have any. I would think about adding javaDoc comments to at least some methods. Eg what does getMaxPower
do? And how is it different from getTotalPower
? You probably know now, but will you remember in a couple of months?
Naming
- Try to be consistent with your names, eg:
AC
vsAirconditioner
- I wouldn't use reverse hungarian notation (but if you do, it should be consistent);
mainCorridorMap
should simply bemainCorridors
. This is especially confusing as there are things called floor maps, but yourfloorMap
field doesn't contain one.
Address
seems like a confusing name. I don't think that you need it at all, but if you do, maybeCorridorLocation
?
clearMotionEvent
: Here, we aren't really dealing with any events. I would probably rename it toturnLightsOff
, andmotionDetected
toturnLightsOn
.
Misc
- Always use curly brackets, even for one-line statements. Not using them can cause bugs. (but if you disagree, you should be consistent)
$endgroup$
$begingroup$
thanks for the review. Answering for "Why does a floor have switchedOFFACs when the corridors already have the ACs? This doesn't seem necessary and is confusing. Also, is there a reason that it is a queue? " Because only the Floor has access to ACs in different corridors. The reason why I kept it in a Queue is, due to some motion event, we may have switched AC1 first. Then comes another event and we are switching off another AC and so on. When we have adequete power at some point, it has been so long since I turned off AC1. So in order to be fair, I put those AC's in queue.
$endgroup$
– Tiny Rick
Oct 19 '17 at 8:17
$begingroup$
Regarding Point number 2 in "Structure".... Great point by the way :) I guess I shoud have a dedicated Collection for Electronic Equipments which can include Lights, ACs, and Security Camera...etc. That collections gets updated during the Corridor object constructor. Then all the powerconsumption computing methods can just refer that collection.
$endgroup$
– Tiny Rick
Oct 19 '17 at 8:27
add a comment |
$begingroup$
Not a full review, but some initial thoughts:
Structure
- You define
SPECPOWER
, but still hardcode the value inside methods. This will surely lead to bugs if you decide to change only the field. - Is there equipment that can be on or off, but which doesn't consume power? If not, I would pull power into equipment (and rename it). If yes, I would add a generic electronic device class. Either will get rid of the duplicate
getPowerConsumption
implementation. - I don't like that
getTotalPowerConsumption
and similar methods need to know about the specific devices (lights, airconditioners). If you need to add say a security camera later on, you will need to add it in many places, and forgetting some will lead to bugs. Possible solutions would be to put all devices in a devices list (which might cause performance issues if you only want to get one specific type), or a different structure such as a map. Your constructors would also need to change then. - Why does a floor have
switchedOFFACs
when the corridors already have the ACs? This doesn't seem necessary and is confusing. Also, is there a reason that it is a queue?
Documentation
You don't have any. I would think about adding javaDoc comments to at least some methods. Eg what does getMaxPower
do? And how is it different from getTotalPower
? You probably know now, but will you remember in a couple of months?
Naming
- Try to be consistent with your names, eg:
AC
vsAirconditioner
- I wouldn't use reverse hungarian notation (but if you do, it should be consistent);
mainCorridorMap
should simply bemainCorridors
. This is especially confusing as there are things called floor maps, but yourfloorMap
field doesn't contain one.
Address
seems like a confusing name. I don't think that you need it at all, but if you do, maybeCorridorLocation
?
clearMotionEvent
: Here, we aren't really dealing with any events. I would probably rename it toturnLightsOff
, andmotionDetected
toturnLightsOn
.
Misc
- Always use curly brackets, even for one-line statements. Not using them can cause bugs. (but if you disagree, you should be consistent)
$endgroup$
$begingroup$
thanks for the review. Answering for "Why does a floor have switchedOFFACs when the corridors already have the ACs? This doesn't seem necessary and is confusing. Also, is there a reason that it is a queue? " Because only the Floor has access to ACs in different corridors. The reason why I kept it in a Queue is, due to some motion event, we may have switched AC1 first. Then comes another event and we are switching off another AC and so on. When we have adequete power at some point, it has been so long since I turned off AC1. So in order to be fair, I put those AC's in queue.
$endgroup$
– Tiny Rick
Oct 19 '17 at 8:17
$begingroup$
Regarding Point number 2 in "Structure".... Great point by the way :) I guess I shoud have a dedicated Collection for Electronic Equipments which can include Lights, ACs, and Security Camera...etc. That collections gets updated during the Corridor object constructor. Then all the powerconsumption computing methods can just refer that collection.
$endgroup$
– Tiny Rick
Oct 19 '17 at 8:27
add a comment |
$begingroup$
Not a full review, but some initial thoughts:
Structure
- You define
SPECPOWER
, but still hardcode the value inside methods. This will surely lead to bugs if you decide to change only the field. - Is there equipment that can be on or off, but which doesn't consume power? If not, I would pull power into equipment (and rename it). If yes, I would add a generic electronic device class. Either will get rid of the duplicate
getPowerConsumption
implementation. - I don't like that
getTotalPowerConsumption
and similar methods need to know about the specific devices (lights, airconditioners). If you need to add say a security camera later on, you will need to add it in many places, and forgetting some will lead to bugs. Possible solutions would be to put all devices in a devices list (which might cause performance issues if you only want to get one specific type), or a different structure such as a map. Your constructors would also need to change then. - Why does a floor have
switchedOFFACs
when the corridors already have the ACs? This doesn't seem necessary and is confusing. Also, is there a reason that it is a queue?
Documentation
You don't have any. I would think about adding javaDoc comments to at least some methods. Eg what does getMaxPower
do? And how is it different from getTotalPower
? You probably know now, but will you remember in a couple of months?
Naming
- Try to be consistent with your names, eg:
AC
vsAirconditioner
- I wouldn't use reverse hungarian notation (but if you do, it should be consistent);
mainCorridorMap
should simply bemainCorridors
. This is especially confusing as there are things called floor maps, but yourfloorMap
field doesn't contain one.
Address
seems like a confusing name. I don't think that you need it at all, but if you do, maybeCorridorLocation
?
clearMotionEvent
: Here, we aren't really dealing with any events. I would probably rename it toturnLightsOff
, andmotionDetected
toturnLightsOn
.
Misc
- Always use curly brackets, even for one-line statements. Not using them can cause bugs. (but if you disagree, you should be consistent)
$endgroup$
Not a full review, but some initial thoughts:
Structure
- You define
SPECPOWER
, but still hardcode the value inside methods. This will surely lead to bugs if you decide to change only the field. - Is there equipment that can be on or off, but which doesn't consume power? If not, I would pull power into equipment (and rename it). If yes, I would add a generic electronic device class. Either will get rid of the duplicate
getPowerConsumption
implementation. - I don't like that
getTotalPowerConsumption
and similar methods need to know about the specific devices (lights, airconditioners). If you need to add say a security camera later on, you will need to add it in many places, and forgetting some will lead to bugs. Possible solutions would be to put all devices in a devices list (which might cause performance issues if you only want to get one specific type), or a different structure such as a map. Your constructors would also need to change then. - Why does a floor have
switchedOFFACs
when the corridors already have the ACs? This doesn't seem necessary and is confusing. Also, is there a reason that it is a queue?
Documentation
You don't have any. I would think about adding javaDoc comments to at least some methods. Eg what does getMaxPower
do? And how is it different from getTotalPower
? You probably know now, but will you remember in a couple of months?
Naming
- Try to be consistent with your names, eg:
AC
vsAirconditioner
- I wouldn't use reverse hungarian notation (but if you do, it should be consistent);
mainCorridorMap
should simply bemainCorridors
. This is especially confusing as there are things called floor maps, but yourfloorMap
field doesn't contain one.
Address
seems like a confusing name. I don't think that you need it at all, but if you do, maybeCorridorLocation
?
clearMotionEvent
: Here, we aren't really dealing with any events. I would probably rename it toturnLightsOff
, andmotionDetected
toturnLightsOn
.
Misc
- Always use curly brackets, even for one-line statements. Not using them can cause bugs. (but if you disagree, you should be consistent)
answered Oct 19 '17 at 7:20
timtim
24.3k22375
24.3k22375
$begingroup$
thanks for the review. Answering for "Why does a floor have switchedOFFACs when the corridors already have the ACs? This doesn't seem necessary and is confusing. Also, is there a reason that it is a queue? " Because only the Floor has access to ACs in different corridors. The reason why I kept it in a Queue is, due to some motion event, we may have switched AC1 first. Then comes another event and we are switching off another AC and so on. When we have adequete power at some point, it has been so long since I turned off AC1. So in order to be fair, I put those AC's in queue.
$endgroup$
– Tiny Rick
Oct 19 '17 at 8:17
$begingroup$
Regarding Point number 2 in "Structure".... Great point by the way :) I guess I shoud have a dedicated Collection for Electronic Equipments which can include Lights, ACs, and Security Camera...etc. That collections gets updated during the Corridor object constructor. Then all the powerconsumption computing methods can just refer that collection.
$endgroup$
– Tiny Rick
Oct 19 '17 at 8:27
add a comment |
$begingroup$
thanks for the review. Answering for "Why does a floor have switchedOFFACs when the corridors already have the ACs? This doesn't seem necessary and is confusing. Also, is there a reason that it is a queue? " Because only the Floor has access to ACs in different corridors. The reason why I kept it in a Queue is, due to some motion event, we may have switched AC1 first. Then comes another event and we are switching off another AC and so on. When we have adequete power at some point, it has been so long since I turned off AC1. So in order to be fair, I put those AC's in queue.
$endgroup$
– Tiny Rick
Oct 19 '17 at 8:17
$begingroup$
Regarding Point number 2 in "Structure".... Great point by the way :) I guess I shoud have a dedicated Collection for Electronic Equipments which can include Lights, ACs, and Security Camera...etc. That collections gets updated during the Corridor object constructor. Then all the powerconsumption computing methods can just refer that collection.
$endgroup$
– Tiny Rick
Oct 19 '17 at 8:27
$begingroup$
thanks for the review. Answering for "Why does a floor have switchedOFFACs when the corridors already have the ACs? This doesn't seem necessary and is confusing. Also, is there a reason that it is a queue? " Because only the Floor has access to ACs in different corridors. The reason why I kept it in a Queue is, due to some motion event, we may have switched AC1 first. Then comes another event and we are switching off another AC and so on. When we have adequete power at some point, it has been so long since I turned off AC1. So in order to be fair, I put those AC's in queue.
$endgroup$
– Tiny Rick
Oct 19 '17 at 8:17
$begingroup$
thanks for the review. Answering for "Why does a floor have switchedOFFACs when the corridors already have the ACs? This doesn't seem necessary and is confusing. Also, is there a reason that it is a queue? " Because only the Floor has access to ACs in different corridors. The reason why I kept it in a Queue is, due to some motion event, we may have switched AC1 first. Then comes another event and we are switching off another AC and so on. When we have adequete power at some point, it has been so long since I turned off AC1. So in order to be fair, I put those AC's in queue.
$endgroup$
– Tiny Rick
Oct 19 '17 at 8:17
$begingroup$
Regarding Point number 2 in "Structure".... Great point by the way :) I guess I shoud have a dedicated Collection for Electronic Equipments which can include Lights, ACs, and Security Camera...etc. That collections gets updated during the Corridor object constructor. Then all the powerconsumption computing methods can just refer that collection.
$endgroup$
– Tiny Rick
Oct 19 '17 at 8:27
$begingroup$
Regarding Point number 2 in "Structure".... Great point by the way :) I guess I shoud have a dedicated Collection for Electronic Equipments which can include Lights, ACs, and Security Camera...etc. That collections gets updated during the Corridor object constructor. Then all the powerconsumption computing methods can just refer that collection.
$endgroup$
– Tiny Rick
Oct 19 '17 at 8:27
add a comment |
$begingroup$
Thanks for sharing your code.
OOP
OOP doesn't mean to "split up" code into random classes.
The ultimate goal of OOP is to reduce code duplication, improve readability and support reuse as well as extending the code.
Doing OOP means that you follow certain principles which are (among others):
- information hiding / encapsulation
- single responsibility
- separation of concerns
- KISS (Keep it simple (and) stupid.)
- DRY (Don't repeat yourself.)
- "Tell! Don't ask."
- Law of demeter ("Don't talk to strangers!")
One basic rule of thumb is: create a new class if there is a difference in behavior. your classes Light
and AirConditioner
only differ in configuration (the number they return for power consumption when on).
Naming
Finding good names is the hardest part in programming, so always take your time to think about the names of your identifiers.
Naming Conventions
Please read (and follow) the
Java Naming Conventions
E.g.: your methods and variables have names like switchOFF
and isON
which should better be switchOff
and isOn
.
misleading naming
Some of your methods have somewhat misleading names.
E.g.: getMaxPower()
implies the access to an objects property while it in deed calculates the max power. Therefore it should better be named calculatePowerConsuption()
.
$endgroup$
$begingroup$
Thanks Timothy for taking you valuable time to review my code. While I agree on your suggestion on the Naming conventions, I am not quite convinced why Light and AirConditioner should be modelled by a single class just varying by the power meter parameter. The reason I can say is, we have to switch on lights in case of a motion event and switch off ACs. If I model them by a single class I need to get and check the power meter every time. IMHO, I find it more comfortable and readable if we have seperate classes for them.
$endgroup$
– Tiny Rick
Oct 19 '17 at 9:19
1
$begingroup$
@TinyRick "we have to switch on lights in case of a motion event and switch off ACs." Your current implementation does not have this different behavior in theLight
andAirConditioner
classes. As mentioned this classes only differ in a constant value. Of cause you might move this behavior there but with your current design these two classes really are one.
$endgroup$
– Timothy Truckle
Oct 19 '17 at 10:42
add a comment |
$begingroup$
Thanks for sharing your code.
OOP
OOP doesn't mean to "split up" code into random classes.
The ultimate goal of OOP is to reduce code duplication, improve readability and support reuse as well as extending the code.
Doing OOP means that you follow certain principles which are (among others):
- information hiding / encapsulation
- single responsibility
- separation of concerns
- KISS (Keep it simple (and) stupid.)
- DRY (Don't repeat yourself.)
- "Tell! Don't ask."
- Law of demeter ("Don't talk to strangers!")
One basic rule of thumb is: create a new class if there is a difference in behavior. your classes Light
and AirConditioner
only differ in configuration (the number they return for power consumption when on).
Naming
Finding good names is the hardest part in programming, so always take your time to think about the names of your identifiers.
Naming Conventions
Please read (and follow) the
Java Naming Conventions
E.g.: your methods and variables have names like switchOFF
and isON
which should better be switchOff
and isOn
.
misleading naming
Some of your methods have somewhat misleading names.
E.g.: getMaxPower()
implies the access to an objects property while it in deed calculates the max power. Therefore it should better be named calculatePowerConsuption()
.
$endgroup$
$begingroup$
Thanks Timothy for taking you valuable time to review my code. While I agree on your suggestion on the Naming conventions, I am not quite convinced why Light and AirConditioner should be modelled by a single class just varying by the power meter parameter. The reason I can say is, we have to switch on lights in case of a motion event and switch off ACs. If I model them by a single class I need to get and check the power meter every time. IMHO, I find it more comfortable and readable if we have seperate classes for them.
$endgroup$
– Tiny Rick
Oct 19 '17 at 9:19
1
$begingroup$
@TinyRick "we have to switch on lights in case of a motion event and switch off ACs." Your current implementation does not have this different behavior in theLight
andAirConditioner
classes. As mentioned this classes only differ in a constant value. Of cause you might move this behavior there but with your current design these two classes really are one.
$endgroup$
– Timothy Truckle
Oct 19 '17 at 10:42
add a comment |
$begingroup$
Thanks for sharing your code.
OOP
OOP doesn't mean to "split up" code into random classes.
The ultimate goal of OOP is to reduce code duplication, improve readability and support reuse as well as extending the code.
Doing OOP means that you follow certain principles which are (among others):
- information hiding / encapsulation
- single responsibility
- separation of concerns
- KISS (Keep it simple (and) stupid.)
- DRY (Don't repeat yourself.)
- "Tell! Don't ask."
- Law of demeter ("Don't talk to strangers!")
One basic rule of thumb is: create a new class if there is a difference in behavior. your classes Light
and AirConditioner
only differ in configuration (the number they return for power consumption when on).
Naming
Finding good names is the hardest part in programming, so always take your time to think about the names of your identifiers.
Naming Conventions
Please read (and follow) the
Java Naming Conventions
E.g.: your methods and variables have names like switchOFF
and isON
which should better be switchOff
and isOn
.
misleading naming
Some of your methods have somewhat misleading names.
E.g.: getMaxPower()
implies the access to an objects property while it in deed calculates the max power. Therefore it should better be named calculatePowerConsuption()
.
$endgroup$
Thanks for sharing your code.
OOP
OOP doesn't mean to "split up" code into random classes.
The ultimate goal of OOP is to reduce code duplication, improve readability and support reuse as well as extending the code.
Doing OOP means that you follow certain principles which are (among others):
- information hiding / encapsulation
- single responsibility
- separation of concerns
- KISS (Keep it simple (and) stupid.)
- DRY (Don't repeat yourself.)
- "Tell! Don't ask."
- Law of demeter ("Don't talk to strangers!")
One basic rule of thumb is: create a new class if there is a difference in behavior. your classes Light
and AirConditioner
only differ in configuration (the number they return for power consumption when on).
Naming
Finding good names is the hardest part in programming, so always take your time to think about the names of your identifiers.
Naming Conventions
Please read (and follow) the
Java Naming Conventions
E.g.: your methods and variables have names like switchOFF
and isON
which should better be switchOff
and isOn
.
misleading naming
Some of your methods have somewhat misleading names.
E.g.: getMaxPower()
implies the access to an objects property while it in deed calculates the max power. Therefore it should better be named calculatePowerConsuption()
.
answered Oct 19 '17 at 8:51
Timothy TruckleTimothy Truckle
4,943416
4,943416
$begingroup$
Thanks Timothy for taking you valuable time to review my code. While I agree on your suggestion on the Naming conventions, I am not quite convinced why Light and AirConditioner should be modelled by a single class just varying by the power meter parameter. The reason I can say is, we have to switch on lights in case of a motion event and switch off ACs. If I model them by a single class I need to get and check the power meter every time. IMHO, I find it more comfortable and readable if we have seperate classes for them.
$endgroup$
– Tiny Rick
Oct 19 '17 at 9:19
1
$begingroup$
@TinyRick "we have to switch on lights in case of a motion event and switch off ACs." Your current implementation does not have this different behavior in theLight
andAirConditioner
classes. As mentioned this classes only differ in a constant value. Of cause you might move this behavior there but with your current design these two classes really are one.
$endgroup$
– Timothy Truckle
Oct 19 '17 at 10:42
add a comment |
$begingroup$
Thanks Timothy for taking you valuable time to review my code. While I agree on your suggestion on the Naming conventions, I am not quite convinced why Light and AirConditioner should be modelled by a single class just varying by the power meter parameter. The reason I can say is, we have to switch on lights in case of a motion event and switch off ACs. If I model them by a single class I need to get and check the power meter every time. IMHO, I find it more comfortable and readable if we have seperate classes for them.
$endgroup$
– Tiny Rick
Oct 19 '17 at 9:19
1
$begingroup$
@TinyRick "we have to switch on lights in case of a motion event and switch off ACs." Your current implementation does not have this different behavior in theLight
andAirConditioner
classes. As mentioned this classes only differ in a constant value. Of cause you might move this behavior there but with your current design these two classes really are one.
$endgroup$
– Timothy Truckle
Oct 19 '17 at 10:42
$begingroup$
Thanks Timothy for taking you valuable time to review my code. While I agree on your suggestion on the Naming conventions, I am not quite convinced why Light and AirConditioner should be modelled by a single class just varying by the power meter parameter. The reason I can say is, we have to switch on lights in case of a motion event and switch off ACs. If I model them by a single class I need to get and check the power meter every time. IMHO, I find it more comfortable and readable if we have seperate classes for them.
$endgroup$
– Tiny Rick
Oct 19 '17 at 9:19
$begingroup$
Thanks Timothy for taking you valuable time to review my code. While I agree on your suggestion on the Naming conventions, I am not quite convinced why Light and AirConditioner should be modelled by a single class just varying by the power meter parameter. The reason I can say is, we have to switch on lights in case of a motion event and switch off ACs. If I model them by a single class I need to get and check the power meter every time. IMHO, I find it more comfortable and readable if we have seperate classes for them.
$endgroup$
– Tiny Rick
Oct 19 '17 at 9:19
1
1
$begingroup$
@TinyRick "we have to switch on lights in case of a motion event and switch off ACs." Your current implementation does not have this different behavior in the
Light
and AirConditioner
classes. As mentioned this classes only differ in a constant value. Of cause you might move this behavior there but with your current design these two classes really are one.$endgroup$
– Timothy Truckle
Oct 19 '17 at 10:42
$begingroup$
@TinyRick "we have to switch on lights in case of a motion event and switch off ACs." Your current implementation does not have this different behavior in the
Light
and AirConditioner
classes. As mentioned this classes only differ in a constant value. Of cause you might move this behavior there but with your current design these two classes really are one.$endgroup$
– Timothy Truckle
Oct 19 '17 at 10:42
add a comment |
$begingroup$
how to execute this program on eclipse?
New contributor
$endgroup$
add a comment |
$begingroup$
how to execute this program on eclipse?
New contributor
$endgroup$
add a comment |
$begingroup$
how to execute this program on eclipse?
New contributor
$endgroup$
how to execute this program on eclipse?
New contributor
New contributor
answered 13 mins ago
user196246user196246
1
1
New contributor
New contributor
add a comment |
add a comment |
Thanks for contributing an answer to Code Review Stack Exchange!
- Please be sure to answer the question. Provide details and share your research!
But avoid …
- Asking for help, clarification, or responding to other answers.
- Making statements based on opinion; back them up with references or personal experience.
Use MathJax to format equations. MathJax reference.
To learn more, see our tips on writing great answers.
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
StackExchange.ready(
function () {
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f178255%2fhotel-power-management%23new-answer', 'question_page');
}
);
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown