8
\$\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.

  1. A Hotel can have multiple floors
  2. Each floor can have multiple main corridors and sub corridors
  3. Both main corridor and sub corridor have one light each
  4. Both main and sub corridor lights consume 5 units of power when ON
  5. Both main and sub corridor have independently controllable ACs
  6. Both main and sub corridor ACs consume 10 units of power when ON
  7. All the lights in all the main corridors need to be switched ON
  8. When a motion is detected in one of the sub corridors the corresponding lights need to be switched ON
  9. When there is no motion for more than a minute the sub corridor lights should be switched OFF
  10. 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
  11. 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("\t\t" + 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.

\$\endgroup\$
2
\$\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 vs Airconditioner
  • I wouldn't use reverse hungarian notation (but if you do, it should be consistent); mainCorridorMap should simply be mainCorridors. This is especially confusing as there are things called floor maps, but your floorMap 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, maybe CorridorLocation?
  • clearMotionEvent: Here, we aren't really dealing with any events. I would probably rename it to turnLightsOff, and motionDetected to turnLightsOn.

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
3
\$\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 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

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.