Draft
Conversation
Zabuzard
commented
Dec 1, 2025
Comment on lines
+100
to
+106
| * his method will set the human State to TRAVELING. | ||
| * It will give the human the ID of the elevator it entered. | ||
| * Then it will call the requestDestination method, setting the destination. | ||
| * If the elevator arrived at the destination it will set the State to ARRIVED | ||
| * and the ID to null. | ||
| * @param elevatorPanel the system inside the elevator which provides information | ||
| * about the elevator and can be used to request a destination floor. |
Member
Author
There was a problem hiding this comment.
that javadoc is more repeating what the code does than explaining the method on a high level to users
Zabuzard
commented
Dec 1, 2025
Comment on lines
+138
to
+141
| /** | ||
| * returns the data of the Human class fields. | ||
| * @return the data of the Human class fields. | ||
| */ |
Member
Author
There was a problem hiding this comment.
sorta obsolete javadoc like that
Zabuzard
commented
Dec 1, 2025
| private final int minFloor; | ||
| private final int floorsServed; | ||
| private int currentFloor; | ||
| private List<Integer> destinations; |
Zabuzard
commented
Dec 1, 2025
Comment on lines
+46
to
+50
| /** | ||
| * The unique ID of the elevator. | ||
| * | ||
| * @return the unique ID of the elevator | ||
| */ |
Member
Author
There was a problem hiding this comment.
the method already has javadoc through the interface its coming from (@Override)
Zabuzard
commented
Dec 1, 2025
Comment on lines
+85
to
+87
| * The chosen Elevator will store this destination in it,s arrayList so it knows | ||
| * which direction to move in and when it has arrived. | ||
| * It will not store the same destination twice. |
Member
Author
There was a problem hiding this comment.
a bit too much "implementation details" but ok
Zabuzard
commented
Dec 1, 2025
| // The elevator is supposed to memorize the destination in a way that | ||
| // it can ensure to eventually reach it. | ||
|
|
||
| if(!destinations.contains(destinationFloor)){ |
Member
Author
There was a problem hiding this comment.
contains on arraylist, a bit problematic perf wise (think about big buildings with lots of humans)
but likely not a big deal in this particular situation as the amount of requests is generally fairly small per elevator
Zabuzard
commented
Dec 1, 2025
Comment on lines
+101
to
+104
| // create a method that will store the destinations in order: | ||
| // closest to the current floor. So every time an elevator moved one floor, | ||
| // we have to call this method as wel, because humans entering add to the | ||
| // array. |
Member
Author
There was a problem hiding this comment.
dangling comment. potential sign of use of AI? lol
Zabuzard
commented
Dec 1, 2025
Comment on lines
+107
to
+109
| * Moving the elevator one floor in the correct direction and | ||
| * when it arrived at it,s destination it will remove that floor from | ||
| * it,s arrayList: destinations. |
Member
Author
There was a problem hiding this comment.
bad javadoc: implementation details
Zabuzard
commented
Dec 1, 2025
| return; | ||
| } | ||
|
|
||
| int destination = destinations.get(0); |
Zabuzard
commented
Dec 1, 2025
| } | ||
| int calculateDistance = 0; | ||
|
|
||
| Elevator chosenElevator = this.elevators.get(0); |
Zabuzard
commented
Dec 1, 2025
Comment on lines
+63
to
+81
| if (chosenElevator.getCurrentFloor() < atFloor){ | ||
| calculateDistance = atFloor - chosenElevator.getCurrentFloor(); | ||
| } else if (chosenElevator.getCurrentFloor() > atFloor){ | ||
| calculateDistance = chosenElevator.getCurrentFloor() - atFloor; | ||
| } | ||
|
|
||
| for (Elevator elevator : elevators) { | ||
|
|
||
| int distance = 0; | ||
| if (elevator.getCurrentFloor() < atFloor){ | ||
| distance = atFloor - elevator.getCurrentFloor(); | ||
| } else if (elevator.getCurrentFloor() > atFloor){ | ||
| distance = elevator.getCurrentFloor() - atFloor; | ||
| } | ||
|
|
||
| if (calculateDistance > distance) { | ||
| calculateDistance = distance; | ||
| chosenElevator = elevator; | ||
| } |
Member
Author
There was a problem hiding this comment.
logic duplication. could be rewritten to pick it in a single loop without that extra logic beforehand
Zabuzard
commented
Dec 1, 2025
Member
Author
Zabuzard
left a comment
Zabuzard
left a comment
There was a problem hiding this comment.
working solution, the logic is overall a bit weak but totally works:
- elevator system assigns closest elevator (based on distance, not travel dir)
- humans enter/exit whenever elevator is at desired floor
- elevators keep a queue of floor requests that they process one by one. problematic with the implementation is that if later requests are already covered by earlier requests that it wont clear them (e.g. at floor 5 going to requested floor 1 should clear a floor 3 request as it moves through it, but doesnt). that leads to elevators having a lot of unecessary traffic. but whatever, it works
- code quality: added javadoc everywhere, cool, but the javadoc is generally of rather low quality/obsolete and often too implementation detailed; variable naming is okay
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Submission by user D