-
Notifications
You must be signed in to change notification settings - Fork 0
Feature/optimize day 09 #52
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
99fab1a
a45105e
a0b8cca
7bc8bb6
ebd22d3
b43a960
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,31 +3,39 @@ | |
| import module java.base; | ||
| import lombok.EqualsAndHashCode; | ||
| import lombok.Getter; | ||
| import org.jetbrains.annotations.NotNull; | ||
|
|
||
| @Getter | ||
| @EqualsAndHashCode | ||
| public class Segment1D { | ||
| public class Segment1D implements Comparable<Segment1D> { | ||
|
|
||
| private Point up; | ||
| private Point down; | ||
| private Point left; | ||
| private Point right; | ||
| private boolean isVertical; | ||
| private final Point first; | ||
| private final Point second; | ||
| private final int minX; | ||
| private final int maxX; | ||
| private final int minY; | ||
| private final int maxY; | ||
| private final boolean isVertical; | ||
|
|
||
|
Comment on lines
+10
to
19
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. issue (bug_risk): The natural ordering of Segment1D is inconsistent with its equality and with the custom comparators, which can cause subtle TreeSet/TreeMap issues.
|
||
| public Segment1D(Point first, Point second) { | ||
| public Segment1D(Point a, Point b) { | ||
| first = a; | ||
| second = b; | ||
| minX = Math.min(first.x(), second.x()); | ||
| maxX = Math.max(first.x(), second.x()); | ||
| minY = Math.min(first.y(), second.y()); | ||
| maxY = Math.max(first.y(), second.y()); | ||
| isVertical = first.x() == second.x(); | ||
| } | ||
|
|
||
| @Override | ||
| public int compareTo(@NotNull Segment1D other) { | ||
|
|
||
| if(first.x() == second.x()) { | ||
| // Vertical segment | ||
| up = new Point(first.x(), Math.max(first.y(), second.y())); | ||
| down = new Point(first.x(), Math.min(first.y(), second.y())); | ||
| isVertical = true; | ||
| } else if(first.y() == second.y()) { | ||
| // Horizontal segment | ||
| left = new Point(Math.min(first.x(), second.x()), first.y()); | ||
| right = new Point(Math.max(first.x(), second.x()), first.y()); | ||
| isVertical = false; | ||
| // Order first by lowest 'x' coordinate | ||
| if(minX != other.minX) { | ||
| return Integer.compare(minX, other.minX); | ||
| } | ||
|
|
||
| // Order by lowest 'y' coordinate | ||
| return Integer.compare(minY, other.minY); | ||
|
Comment on lines
+35
to
+39
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 MEDIUM RISK The |
||
| } | ||
|
|
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| package com.adventofcode.flashk.day09.refactor; | ||
|
|
||
| import module java.base; | ||
| import com.adventofcode.flashk.day09.Segment1D; | ||
|
|
||
| public class ComparatorSegment1DHorizontal implements Comparator<Segment1D> { | ||
|
|
||
| @Override | ||
| public int compare(Segment1D o1, Segment1D o2) { | ||
|
|
||
| if(o1.getMinX() != o2.getMinX()) { | ||
| return Integer.compare(o1.getMinX(), o2.getMinX()); | ||
| } | ||
|
|
||
| return Integer.compare(o1.getMinY(), o2.getMinY()); | ||
| } | ||
|
Comment on lines
+6
to
+16
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ⚪ LOW RISK Suggestion: The |
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| package com.adventofcode.flashk.day09.refactor; | ||
|
|
||
| import static java.lang.IO.println; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ⚪ LOW RISK Nitpick: The import |
||
|
|
||
| import module java.base; | ||
| import com.adventofcode.flashk.day09.Segment1D; | ||
|
|
||
| public class ComparatorSegment1DVertical implements Comparator<Segment1D> { | ||
|
|
||
| @Override | ||
| public int compare(Segment1D o1, Segment1D o2) { | ||
|
|
||
| if(o1.getMinY() != o2.getMinY()) { | ||
| return Integer.compare(o1.getMinY(), o2.getMinY()); | ||
| } | ||
|
|
||
| return Integer.compare(o1.getMinX(), o2.getMinX()); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,50 @@ | ||
| package com.adventofcode.flashk.day09.refactor; | ||
|
|
||
| import module java.base; | ||
| import com.adventofcode.flashk.day09.EventType1D; | ||
| import com.adventofcode.flashk.day09.Segment1D; | ||
| import lombok.AllArgsConstructor; | ||
| import lombok.Getter; | ||
| import org.jetbrains.annotations.NotNull; | ||
|
|
||
| @Getter | ||
| public class Event1DRefactor implements Comparable<Event1DRefactor> { | ||
|
|
||
| /// The exact coordinate that triggered the event. | ||
| /// | ||
| /// Can be either an `x` or an `y` coordinate. | ||
| private final int coordinate; | ||
|
|
||
| /// The segment that triggered the event. | ||
| private Segment1D segment; // The segment this coordinate event belongs to. | ||
|
|
||
| /// The rectangle that triggered the event. | ||
| private Rectangle rectangle; | ||
|
|
||
| /// The {@link EventType1D type} of event. | ||
| private final EventType1D type; | ||
|
|
||
| public Event1DRefactor(int coordinate, Segment1D segment, EventType1D type) { | ||
| this.coordinate = coordinate; | ||
| this.segment = segment; | ||
| this.type = type; | ||
| } | ||
|
|
||
| public Event1DRefactor(int coordinate, Rectangle rectangle, EventType1D type) { | ||
| this.coordinate = coordinate; | ||
| this.rectangle = rectangle; | ||
| this.type = type; | ||
| } | ||
|
|
||
| @Override | ||
| public int compareTo(@NotNull Event1DRefactor other) { | ||
|
|
||
| // Priority on lower coordinate | ||
| if(this.coordinate != other.coordinate) { | ||
| return Integer.compare(this.coordinate, other.coordinate); | ||
| } | ||
|
|
||
| // In case of being same coordinates, give priority depending on the event type | ||
| return Integer.compare(this.type.getPriority(), other.type.getPriority()); | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
issue (bug_risk): Segment1D.compareTo is inconsistent with @EqualsAndHashCode and may break TreeSet-based logic.
compareTo only uses minX and minY, while Lombok’s @EqualsAndHashCode includes all fields (first, second, minX, maxX, minY, maxY, isVertical). This allows two non-equal segments to compare as 0, violating the Comparable contract and breaking TreeSet behavior (dropped entries, wrong ceiling/floor). Please either make compareTo fully consistent with equals or remove @EqualsAndHashCode and define equals/hashCode to match the ordering semantics.