-
Notifications
You must be signed in to change notification settings - Fork 1
Description
To są tylko bardzo drobne (nie zbyt ważne) propozycje. Nie musicie je stosować jeśli macie dobry powód, albo nawet jeśli wam po prostu nie podoba się 😃
JGeo/src/main/java/Project/model/BasicLine.java
Lines 30 to 32 in 4eeb487
| public double distance(BasicPoint p) { | |
| return Math.abs(A * p.x + B * p.y + C) / Math.sqrt(A * A + B * B); | |
| } |
Czy to nie jest po prostu
BasicLine.distance(p, this)?
Mam nadzieję, że jesteście świadomi tego, że basicSegment.distance(point) nie zawsze zwraca poprawną wartość (to trochę naruszą L w SOLID, ale w tym przypadku można po prostu nadpisać ten distance tak aby rzucał wyjątek)
JGeo/src/main/java/Project/view/ViewablePlane.java
Lines 63 to 81 in 4eeb487
| public List<ViewableShape> getClickedShapesList(double screenX, double screenY) { | |
| List<ViewableShape> clickedShapes = new ArrayList<>(); | |
| for (ViewableShape shape : shapes) | |
| if (shape.hasPoint(screenX, screenY)) | |
| clickedShapes.add(shape); | |
| clickedShapes.sort(ViewableShape.getPriorityComparator()); | |
| return clickedShapes; | |
| } | |
| public List<ViewableShape> getClickedShapesList(double screenX, double screenY, Predicate<ViewableShape> predicate) { | |
| List<ViewableShape> clickedShapes = new ArrayList<>(); | |
| for (ViewableShape shape : shapes) | |
| if (predicate.test(shape) && shape.hasPoint(screenX, screenY)) | |
| clickedShapes.add(shape); | |
| clickedShapes.sort(ViewableShape.getPriorityComparator()); | |
| return clickedShapes; | |
| } |
To jest dobry usecase na strumienie:
shapes.stream().filter(shape -> shape.hasPoint()).sorted(getPriorityCmp()).toList()A w ogóle, aby nie duplikować kodu, ta pierwsza metoda może wołać
getClickedShapes(x, y, () -> true)Tak naprawdę te wszystkie 4 metody
getClicked...() mają współdzielony kod (shapes.stream().filter(predicate && hasPoint))
JGeo/src/main/java/Project/Config.java
Line 21 in 4eeb487
| public static void load() { |
to wszystko można wsadzić do tzw statycznego bloku
static { ... }wtedy konfiguracja się załaduje sama, bez potrzeby wołania
load(). I dodatkowo to pozwoli zrobić wszystkie zmienne Configa finalnymi
JGeo/src/main/java/Project/controller/builders/circles/IncircleBuilder.java
Lines 74 to 85 in 4eeb487
| public static BasicCircle getCircle(BasicPoint a, BasicPoint b, BasicPoint c) { | |
| double AB = a.distance(b); | |
| double BC = b.distance(c); | |
| double CA = c.distance(a); | |
| double p = AB + BC + CA; | |
| double x = (BC * a.x + CA * b.x + AB * c.x) / p; | |
| double y = (BC * a.y + CA * b.y + AB * c.y) / p; | |
| double s = p / 2; | |
| double r = Math.sqrt((s - AB) * (s - BC) * (s - CA) / s); | |
| BasicPoint center = new BasicPoint(x, y); | |
| return new BasicCircle(center, r); | |
| } |
takiego typu pomocnicze metody chyba jednak należą do modelu
JGeo/src/main/java/Project/controller/builders/intersections/IntersectionBuilder.java
Lines 18 to 33 in 4eeb487
| public boolean acceptArgument(GeometricShape shape) { | |
| boolean accepted = false; | |
| if (lineIntersectionBuilder.acceptArgument(shape)) { | |
| accepted = true; | |
| } | |
| if (circleIntersectionBuilder.acceptArgument(shape)) { | |
| accepted = true; | |
| } | |
| if (lineAndCircleIntersectionBuilder.acceptArgument(shape)) { | |
| accepted = true; | |
| } | |
| if (genCircleIntersectionBuilder.acceptArgument(shape)) { | |
| accepted = true; | |
| } | |
| return accepted; | |
| } |
chyba dość męczące jest if-owanie po wszystkich 4 przypadkach w każdej metodzie 😄
co, gdy trzymana jest lista tych 4 builderów, i wtedy coś w stylu
boolean acceptArgument(shape) {
return builders.stream().takeWhile(b -> !b.acceptArgument(shape)) < builders.size();
}
boolean isReady() {
return builders.stream().anyMatch(Builder::isReady);
}
void reset() {
builders.stream().forEach(Builder::reset);
}
boolean awaitsPoint() {
return builders.stream().anyMatch(Builder::awaitsPoint);
}