Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 8 additions & 7 deletions Contest/Assignment/src/org/togetherjava/event/elevator/Main.java
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
package org.togetherjava.event.elevator;

import org.togetherjava.event.elevator.elevators.Elevator;
import org.togetherjava.event.elevator.humans.Human;
import org.togetherjava.event.elevator.simulation.Simulation;

public final class Main {
Expand All @@ -19,10 +17,13 @@ public static void main(final String[] args) {
// Eventually try out the randomly generated systems. If you want to debug a problem you encountered
// with one of them, note down the seed that it prints at the beginning and then use the variant that takes this seed.
// That way, it will generate the same system again, and you can repeat the test.
Simulation simulation = Simulation.createSingleElevatorSingleHumanSimulation();
// Simulation simulation = Simulation.createSimpleSimulation();
// Simulation simulation = Simulation.createRandomSimulation(5, 50, 10);
// Simulation simulation = Simulation.createRandomSimulation(putDesiredSeedHere, 5, 50, 10);
//Simulation simulation = Simulation.createSingleElevatorSingleHumanSimulation();
Simulation simulation = Simulation.createSimpleSimulation();
//Simulation simulation = Simulation.createRandomSimulation(5, 50, 10);
//Simulation simulation = Simulation.createRandomSimulation(4637787693156730566L, 5, 5000, 100);

//Simulation simulation = Simulation.createRandomSimulation(-806872529110342439L, 5, 50, 10);
//Simulation simulation = Simulation.createRandomSimulation(3, 100, 100_000, 100);

simulation.printSummary();

Expand All @@ -35,7 +36,7 @@ public static void main(final String[] args) {
simulation.step();
simulation.prettyPrint();

if (simulation.getStepCount() >= 100_000) {
if (simulation.getStepCount() >= 200000) {
throw new IllegalStateException("Simulation aborted. All humans should have arrived"
+ " by now, but they did not. There is likely a bug in your code.");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,16 @@
*/
public final class Elevator implements ElevatorPanel {
private static final AtomicInteger NEXT_ID = new AtomicInteger(0);
private static final String INVALID_FLOOR_ERROR = "Floor is outside the valid range for this elevator!";

private final int id;
private final int minFloor;
private final int floorsServed;
private final int maxFloor;
private final int[] requests;
Copy link
Member Author

@Zabuzard Zabuzard Dec 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

naming issue, what does int represent? (human IDs? floors?)

private final Object lock = new Object();
private int currentFloor;
private TravelDirection currentDirection;

/**
* Creates a new elevator.
Expand All @@ -37,6 +42,9 @@ public Elevator(int minFloor, int floorsServed, int currentFloor) {
this.minFloor = minFloor;
this.currentFloor = currentFloor;
this.floorsServed = floorsServed;
this.maxFloor = minFloor + floorsServed - 1;
this.currentDirection = TravelDirection.NONE;
requests = new int[floorsServed];
}

@Override
Expand All @@ -54,37 +62,185 @@ public int getFloorsServed() {

@Override
public int getCurrentFloor() {
return currentFloor;
synchronized (lock) {
return currentFloor;
}
}

@Override
public void requestDestinationFloor(int destinationFloor) {
// TODO Implement. This represents a human or the elevator system
// itself requesting this elevator to eventually move to the given floor.
// The elevator is supposed to memorize the destination in a way that
// it can ensure to eventually reach it.
System.out.println("Request for destination floor received");
synchronized (lock) {
addRequest(destinationFloor);
}
Comment on lines +72 to +74
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a bit of excessive placement of sync everywhere. addRequest syncs itself already, so this one for example isnt needed

}

@Override
public TravelDirection getAdvertisedDirection() {
synchronized (lock) {
if (!moreRequestsInCurrentDirection()) {
return computeBestDirection();
}
return currentDirection;
}
}

/**
* Moves the elevator one floor
* Elevator can either: move up one floor, move down one floor, or stand still
*/
public void moveOneFloor() {
// TODO Implement. Essentially there are three possibilities:
// - move up one floor
// - move down one floor
// - stand still
// The elevator is supposed to move in a way that it will eventually reach
// the floors requested by Humans via requestDestinationFloor(), ideally "fast" but also "fair",
// meaning that the average time waiting (either in corridor or inside the elevator)
// is minimized across all humans.
// It is essential that this method updates the currentFloor field accordingly.
System.out.println("Request to move a floor received");
synchronized (lock) {
clearRequests(currentFloor);
if (!moreRequestsInCurrentDirection()) {
currentDirection = computeBestDirection();
}
if (currentDirection == TravelDirection.UP) {
goUp();
} else if (currentDirection == TravelDirection.DOWN) {
goDown();
}
}
}

@Override
public synchronized String toString() {
return new StringJoiner(", ", Elevator.class.getSimpleName() + "[", "]").add("id=" + id)
.add("minFloor=" + minFloor)
.add("floorsServed=" + floorsServed)
.add("currentFloor=" + currentFloor)
.toString();
public String toString() {
synchronized (lock) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(obsolete)

return new StringJoiner(", ", Elevator.class.getSimpleName() + "[", "]").add("id=" + id)
.add("minFloor=" + minFloor)
.add("floorsServed=" + floorsServed)
.add("currentFloor=" + currentFloor)
.toString();
}
}

/**
* Determines if there are additional requests in currentDirection in requests array for this elevator
*
* @return a boolean (true means there are more requests in current direction, otherwise false)
*/
private boolean moreRequestsInCurrentDirection() {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

name should start with is or are or something like that

synchronized (lock) {
if (currentDirection == TravelDirection.NONE) {
return getRequestCount(currentFloor) > 0;
}
if (currentDirection == TravelDirection.UP) {
return countRequestsAbove() > 0;
}
if (currentDirection == TravelDirection.DOWN) {
return countRequestsBelow() > 0;
}
throw new RuntimeException("currentDirection is in an invalid state!");
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would be useful to add the current state to the error message

}
}

/**
* Determines the best direction to travel based on request count in each direction
*
* @return the desired TravelDirection to move in
*/
private TravelDirection computeBestDirection() {
synchronized (lock) {
int requestsAbove = countRequestsAbove();
int requestsBelow = countRequestsBelow();
if (requestsAbove == 0 && requestsBelow == 0) {
return TravelDirection.NONE;
}
return (requestsAbove >= requestsBelow) ? TravelDirection.UP : TravelDirection.DOWN;
}
}

/**
* Counts the requests above the currentFloor for the current elevator instance
*
* @return an integer count of requests above the current floor
*/
private int countRequestsAbove() {
synchronized (lock) {
int requests = 0;
for (int i = currentFloor + 1; i <= maxFloor; i++) {
requests += getRequestCount(i);
}
return requests;
}
}

/**
* Counts the requests below the currentFloor for the current elevator instance
*
* @return an integer count of requests below the current floor
*/
private int countRequestsBelow() {
synchronized (lock) {
int requests = 0;
for (int i = currentFloor - 1; i >= minFloor; i--) {
requests += getRequestCount(i);
}
return requests;
}
}

/**
* Moves the elevator up one floor
*/
private void goUp() {
synchronized (lock) {
currentFloor += 1;
}
}

/**
* Moves the elevator down one floor
*/
private void goDown() {
synchronized (lock) {
currentFloor -= 1;
}
}
Comment on lines +185 to +198
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no protection for the floor range


/**
* Adds a request to the current elevator for the desired floor. Should be used alongside
* {@link Elevator#getRequestCount(int)} to retrieve the counts to ensure offsets remain consistent.
*
* @param floor the floor the request is for
* @throws IllegalArgumentException if floor is outside valid range
*/
private void addRequest(int floor) {
if (floor < minFloor || floor > maxFloor) {
throw new IllegalArgumentException(INVALID_FLOOR_ERROR);
}
synchronized (lock) {
requests[floor - minFloor] += 1;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

floor - minFloor, would be useful to put that into a well named var

}
}

/**
* Gets the number of requests from the current elevator for a given floor
*
* @param floor the floor to get the request count from
* @return an integer with the request count
* @throws IllegalArgumentException if floor is outside valid range
*/
private int getRequestCount(int floor) {
if (floor < minFloor || floor > maxFloor) {
throw new IllegalArgumentException(INVALID_FLOOR_ERROR);
}
synchronized (lock) {
return requests[floor - minFloor];
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(floor - minFloor, would be useful to put that into a well named var)

}
}

/**
* Removes requests for a given floor
*
* @param floor the floor to clear requests from
* @throws IllegalArgumentException if floor is outside valid range
*/
private void clearRequests(int floor) {
if (floor < minFloor || floor > maxFloor) {
throw new IllegalArgumentException(INVALID_FLOOR_ERROR);
}
synchronized (lock) {
requests[floor - minFloor] = 0;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

floor - minFloor, would be useful to put that into a well named var.

since its repeated 3 times now, maybe even a helper method

}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,4 +25,11 @@ public interface ElevatorPanel {
* @param destinationFloor the desired destination, must be within the range served by this elevator
*/
void requestDestinationFloor(int destinationFloor);

/**
* Shows the "advertised" direction where the elevator is heading so passengers know whether to board.
*
* @return a TravelDirection that represents the direction this elevator is headed
*/
TravelDirection getAdvertisedDirection();
}
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package org.togetherjava.event.elevator.elevators;

import org.togetherjava.event.elevator.humans.ElevatorListener;

import java.util.ArrayList;
import java.util.List;

Expand Down Expand Up @@ -33,17 +32,66 @@ public void ready() {

@Override
public void requestElevator(int atFloor, TravelDirection desiredTravelDirection) {
// TODO Implement. This represents a human standing in the corridor,
// requesting that an elevator comes to pick them up for travel into the given direction.
// The system is supposed to make sure that an elevator will eventually reach this floor to pick up the human.
// The human can then enter the elevator and request their actual destination within the elevator.
// Ideally this has to select the best elevator among all which can reduce the time
// for the human spending waiting (either in corridor or in the elevator itself).
System.out.println("Request for elevator received");
findNearestInDesiredDirection(elevators, atFloor, desiredTravelDirection).requestDestinationFloor(atFloor);
}

public void moveOneFloor() {
elevators.forEach(Elevator::moveOneFloor);
elevators.forEach(elevator -> elevatorListeners.forEach(listener -> listener.onElevatorArrivedAtFloor(elevator)));
elevators.parallelStream().forEach(Elevator::moveOneFloor);
elevators.parallelStream().forEach(elevator -> elevatorListeners.parallelStream().forEach(listener -> listener.onElevatorArrivedAtFloor(elevator)));
}

/**
* Helper method that finds the nearest elevator to a given floor in position to travel in the desired direction.
*
* @param elevators the list of elevators
* @param floor the desired floor (where the human resides)
* @param desiredDirection the desired TravelDirection
* @return elevator object representing the nearest in that direction OR a fallback to the nearest in general if
* none fit the direction criteria
*/
private static Elevator findNearestInDesiredDirection(List<Elevator> elevators, int floor, TravelDirection desiredDirection) {
List<Elevator> directionMatchedElevators = new ArrayList<>();
for (Elevator e : elevators) {
if (desiredDirection == TravelDirection.DOWN) {
if (e.getCurrentFloor() > floor) {
directionMatchedElevators.add(e);
}
} else if (desiredDirection == TravelDirection.UP) {
if (e.getCurrentFloor() < floor){
directionMatchedElevators.add(e);
}
}
}
Elevator result;
if (!directionMatchedElevators.isEmpty()) {
result = findNearestElevator(directionMatchedElevators, floor);
} else {
result = findNearestElevator(elevators, floor);
}
if (result == null) {
throw new RuntimeException("Found no elevators < Integer.MAX_VALUE distance away from complete list of elevators!");
Copy link
Member Author

@Zabuzard Zabuzard Dec 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that "< Integer.MAX_VALUE distance" part is an assumption of implementation detail that isnt up to this method to make but only the find-method. comments like that easily drift apart from the implementation and rot

}
return result;
}

/**
* A helper method that returns the nearest elevator to the given floor.
* NOTE: Returned elevator can be {@code null} (be careful if list can be empty)
*
* @param elevators the list of elevators
* @param floor the floor you want to find the nearest elevator to
* @return the closest Elevator or null if none found
*/
private static Elevator findNearestElevator(List<Elevator> elevators, int floor) {
Elevator nearest = null;
int nearestDistance = Integer.MAX_VALUE;
for (Elevator elevator : elevators) {
int distance = Math.abs(elevator.getCurrentFloor() - floor);
if (distance < nearestDistance) {
nearest = elevator;
nearestDistance = distance;
}
}
return nearest;
Comment on lines +85 to +95
Copy link
Member Author

@Zabuzard Zabuzard Dec 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the method mentions returning null is a thing.
in that case Optional would have been the more appriopriate pick

}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,5 @@
public enum TravelDirection {
UP,
DOWN,
NONE
}
Loading