Submission E by Zabuzard · Pull Request #10 · Together-Java/ElevatorEvent

Zabuzard · 2025-12-01T11:18:36Z

Submission by user E

Zabuzard · 2025-12-01T11:19:21Z

Contest/Assignment/src/org/togetherjava/event/elevator/Main.java

            simulation.prettyPrint();

-            if (simulation.getStepCount() >= 100_000) {
+            if (simulation.getStepCount() >= 100_00) {


_ should be shifted to avoid confusion (10_000)

Zabuzard · 2025-12-01T11:19:48Z

Contest/Assignment/src/org/togetherjava/event/elevator/simulation/Simulation.java

    }
+    public static Simulation createCustomSimulation() {
+        return new Simulation(List.of(new Elevator(1, 10, 10)),
+                List.of(new Human(2, 2),new Human(1, 2)));


missing space after , between the two humans

Zabuzard · 2025-12-01T11:20:56Z

Contest/Assignment/src/org/togetherjava/event/elevator/elevators/ElevatorPanel.java

+
+
+    /**
+     * Counts the number of humans exited the elevator and updates the number of humans aboard counter.


worded a bit too much implementation-detailed. like, that second part of the sentence should just be dropped "updates the number of humans aboard counter", thats implementation details that might change and will eventually rot

Zabuzard · 2025-12-01T11:21:34Z

Contest/Assignment/src/org/togetherjava/event/elevator/elevators/ElevatorPanel.java

+    void exitedElevator();
+
+    /**
+     * Counts the number of humans entered the elevator.
+     */
+    void enteredElevator();


fair addition. (but IRL elevators usually dont really have an awareness for how many humans are aboard)

Zabuzard · 2025-12-01T11:22:23Z

Contest/Assignment/src/org/togetherjava/event/elevator/humans/Human.java

    private State currentState;
    private final int startingFloor;
    private final int destinationFloor;
+    private boolean requestedDestinationFloor;


should start with sth like has (or is or are etc) so its clearer that its a boolean

Zabuzard · 2025-12-01T11:23:29Z

Contest/Assignment/src/org/togetherjava/event/elevator/humans/Human.java

+        if(startingFloor - destinationFloor > 0) travelDirection = TravelDirection.UP;
+        else travelDirection = TravelDirection.DOWN;


(in java its more common to use curly-braces even on one-liner ifs)

a ternary could also be used here for doing it more compact:

Foo foo = condition ? A : B;

Zabuzard · 2025-12-01T11:28:36Z

Contest/Assignment/src/org/togetherjava/event/elevator/humans/Human.java

+        if(currentEnteredElevatorId == null) {
+            if (currentState.equals(State.WAITING_FOR_ELEVATOR) && elevatorPanel.getCurrentFloor() == this.getStartingFloor()) {
+                currentEnteredElevatorId = elevatorPanel.getId();
+                if(!requestedDestinationFloor) {
+                    elevatorPanel.requestDestinationFloor(this.destinationFloor);
+                    elevatorPanel.enteredElevator();
+                    requestedDestinationFloor = true;
+                }
+                this.currentState = State.TRAVELING_WITH_ELEVATOR;
+            }
+        }
+        else {
+            if (currentState.equals(State.TRAVELING_WITH_ELEVATOR) && elevatorPanel.getId() == this.currentEnteredElevatorId && elevatorPanel.getCurrentFloor() == this.getDestinationFloor()) {
+                this.currentState = State.ARRIVED;
+                this.currentEnteredElevatorId = null;
+                elevatorPanel.exitedElevator();
+                System.out.println("Arrived-event received");
+            }
+        }


this is coded a bit too defensively and hence its easy to not notice bugs. better would be to change the structure to check on the state and do the rest as assertions:

if (currentState == State.WAITING_FOR_ELEVATOR) { if (currentEnteredElevatorId != null) throw IllegalStateException("cant be in an elevator when waiting for one..."); ... } else if (currentState == State.TRAVELING_WITH_ELEVATOR) { if (something is wrong...) throw ... ... }

(doing it based on the states is often called "state-machine")

Zabuzard · 2025-12-01T11:29:17Z

Contest/Assignment/src/org/togetherjava/event/elevator/humans/Human.java

        //  arrival at the human's destination floor, the human can now exit the elevator.
-        System.out.println("Arrived-event received");
+        if(currentEnteredElevatorId == null) {
+            if (currentState.equals(State.WAITING_FOR_ELEVATOR) && elevatorPanel.getCurrentFloor() == this.getStartingFloor()) {


enums can and should be checked with ==

Zabuzard · 2025-12-01T11:30:03Z

Contest/Assignment/src/org/togetherjava/event/elevator/elevators/Elevator.java

+        if(destinationFloor > currentFloor) travellingDirection = TravelDirection.UP;
+        else if(destinationFloor < currentFloor) travellingDirection = TravelDirection.DOWN;
+        else travellingDirection = null;


(add curlies, also spaces around the if (...))

Zabuzard · 2025-12-01T11:30:54Z

Contest/Assignment/src/org/togetherjava/event/elevator/elevators/Elevator.java

+     * The idea is to make the elevator go to the nearest or farthest request based on strategy.
+     */
+
+    private void sortingList() {


method naming could be improved. for example sortRequestsByPriority

Zabuzard · 2025-12-01T11:31:23Z

Contest/Assignment/src/org/togetherjava/event/elevator/elevators/Elevator.java

+     * It is possible that the requests ends for the current elevator but there are still humans onboard.
+     * In that case the paternoster is the last resort to make sure the humans onboard reach the destination floor.


how is that possible? lol.
but great that u added a fallback

Zabuzard · 2025-12-01T11:32:21Z

Contest/Assignment/src/org/togetherjava/event/elevator/elevators/Elevator.java

+     * It is possible that the requests ends for the current elevator but there are still humans onboard.
+     * In that case the paternoster is the last resort to make sure the humans onboard reach the destination floor.
+     */
+    private void lastResort() {


method naming. maybe moveOneFloorByFallbackStrategy

Zabuzard · 2025-12-01T11:33:18Z

Contest/Assignment/src/org/togetherjava/event/elevator/elevators/Elevator.java

+            return;
+        }
+        sortingList();
+        int currentRequest = requestedDestinationFloors.getLast();


a bit unconventional to put the "best" request at the end instead of the front

Zabuzard · 2025-12-01T11:34:03Z

Contest/Assignment/src/org/togetherjava/event/elevator/elevators/Elevator.java

+        sortingList();
+        int currentRequest = requestedDestinationFloors.getLast();


design-wise this is a bit difficult to understand that it belongs together:

sortFoo(); int foo = foos.getLast();

something better would be if it comes out of that method, so something like:

int foo = pickBestFoo();

Zabuzard · 2025-12-01T11:34:57Z

Contest/Assignment/src/org/togetherjava/event/elevator/elevators/Elevator.java

+     * But the request for the current floor can make the elevator come to the current floor again or stay at it for consecutive steps.
+     * To prevent such loss of steps, this method removes the number of request according to the number of humans exited.
+     */
+    private void removeRequests() {


method naming. its unclear which requests it will remove. from the name it sounds like it clears all of them

Zabuzard · 2025-12-01T11:35:47Z

Contest/Assignment/src/org/togetherjava/event/elevator/elevators/Elevator.java

+        for(int i = 0; i < humanExited; i++) {
+            for(int j = 0; j < requestedDestinationFloors.size(); j++) {
+                if(requestedDestinationFloors.get(j) == currentFloor) {
+                    requestedDestinationFloors.remove(j);
+                    humanExited--;
+                    j--;
+                }
+            }
+        }


that logic looks a bit wobbly. especially with that outer loop, i isnt used anywhere either. smells a bit.

Zabuzard · 2025-12-01T11:36:13Z

Contest/Assignment/src/org/togetherjava/event/elevator/elevators/Elevator.java

+import java.util.*;
 import java.util.concurrent.atomic.AtomicInteger;

+import static org.togetherjava.event.elevator.elevators.ElevatorSystem.mod;


prefer to not use static imports, it typically confuses readers

Zabuzard · 2025-12-01T11:37:24Z

Contest/Assignment/src/org/togetherjava/event/elevator/elevators/ElevatorSystem.java

+    static int mod(int num) {
+        int modValue = num;
+        if(modValue < 0) modValue *= -1;
+        return modValue;
+    }


not quite sure what that is supposed to do. looks like Math.abs(x)? (abs = absolute).
the name mod sounds more like modulo (%)

Zabuzard · 2025-12-01T11:38:38Z

Contest/Assignment/src/org/togetherjava/event/elevator/elevators/Elevator.java

+        for(int i = 0; i < requestedDestinationFloors.size() - 1; i++) {
+            if(mod(requestedDestinationFloors.get(i) - currentFloor) < mod(requestedDestinationFloors.get(i+1)) - currentFloor) {
+                int temp = requestedDestinationFloors.get(i + 1);
+                requestedDestinationFloors.remove(i + 1);
+                requestedDestinationFloors.add(i + 1,requestedDestinationFloors.get(i));
+                requestedDestinationFloors.remove(i);
+                requestedDestinationFloors.add(i,temp);
+            }
+        }


that looks a bit wobbly. and its also executed in every single step, so it probably contributes to some simulation slowdown.

editing a list while you are iterating it is also dangerous, can easily blow up and lead to infinite loops

Zabuzard

working submission. the logic is a bit wobbly, potentially has some smaller bugs but its a decent attempt at improving the logic from something more basic

humans enter/exit when elevators are at their corresponding floors
elevator system calls the closest elevator that is going into the desired direction
elevators move based on a list of requested floors. there is an attempt to sort/group these so they align in direction for an optimal order to serve them, great. the logic is a bit wobbly overall though (potentially has bugs), a fallback paternoster logic was added to cover cases where the code goes bonkers. great save, haha
code quality: overall good, great javadoc as well, helper methods as well (could have better names though)

Submission

492ad3f

Zabuzard added submission event_november_2025 labels Dec 1, 2025

Zabuzard commented Dec 1, 2025

View reviewed changes



		/**
		* Counts the number of humans exited the elevator and updates the number of humans aboard counter.

		if(startingFloor - destinationFloor > 0) travelDirection = TravelDirection.UP;
		else travelDirection = TravelDirection.DOWN;

		* It is possible that the requests ends for the current elevator but there are still humans onboard.
		* In that case the paternoster is the last resort to make sure the humans onboard reach the destination floor.

		sortingList();
		int currentRequest = requestedDestinationFloors.getLast();

Conversation

Zabuzard commented Dec 1, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Zabuzard left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant