patternjavaModerate
Shamos-Hoey algorithm for checking the self-intersection of a closed shape
Viewed 0 times
thecheckingclosedalgorithmhoeyforshamosintersectionshapeself
Problem
I implemented the Shamos-Hoey algorithm to check if a closed shape is self-intersected. Is this algorithm ok in terms of performance?
```
public boolean isSelfIntersected() {
Set plines = new HashSet();
for (Path2D ps : this.getPath()) {
PathIterator p_it = ps.getPathIterator(null, /flatness/ 1);
List estPath = new ArrayList();
while (!p_it.isDone()) {
p_it.next();
double[] coords = new double[6];
int s = p_it.currentSegment(coords);
if (s == PathIterator.SEG_LINETO) {
if (estPath.size() != 0) {
Point2D pp = estPath.get(estPath.size() - 1).getP2();
estPath.add(new Line2D.Double(pp, new Point2D.Double(coords[0],coords[1])));
} else {
estPath.add(new Line2D.Double(new Point2D.Double(), new Point2D.Double(coords[0],coords[1])));
}
}
}
for (Line2D lq : estPath) {
plines.add(tweakLine(lq));
}
}
return ShamosHoeyAlgorithm(plines);
}
/**
* Moves first point of the line by 0.0000001 of it's length.
* @return
*/
static Line2D tweakLine(Line2D l) {
Line2D ql = new Line2D.Double(
l.getX1() + 0.0000001*(l.getX2() - l.getX1()),
l.getY1() + 0.0000001*(l.getY2() - l.getY1()),
l.getX2() - 0.0000001*(l.getX2() - l.getX1()),
l.getY2() - 0.0000001*(l.getY2() - l.getY1()));
return ql;
}
public class ShamosHoeyAlgorithm {
public static boolean ShamosHoeyAlgorithm(Collection lines) {
List events = new ArrayList(lines.size() * 2);
for (Line2D li : lines) {
if (li.getX1() li.getX2()) {
Line2D l = new Line2D.Double(li.getP2(), li.getP1());
events.add(new AlgEvent(l, true));
events.add(new AlgEvent(l, false));
} else {
if (li.getY1() li.getY2()) {
Line2D l = n
```
public boolean isSelfIntersected() {
Set plines = new HashSet();
for (Path2D ps : this.getPath()) {
PathIterator p_it = ps.getPathIterator(null, /flatness/ 1);
List estPath = new ArrayList();
while (!p_it.isDone()) {
p_it.next();
double[] coords = new double[6];
int s = p_it.currentSegment(coords);
if (s == PathIterator.SEG_LINETO) {
if (estPath.size() != 0) {
Point2D pp = estPath.get(estPath.size() - 1).getP2();
estPath.add(new Line2D.Double(pp, new Point2D.Double(coords[0],coords[1])));
} else {
estPath.add(new Line2D.Double(new Point2D.Double(), new Point2D.Double(coords[0],coords[1])));
}
}
}
for (Line2D lq : estPath) {
plines.add(tweakLine(lq));
}
}
return ShamosHoeyAlgorithm(plines);
}
/**
* Moves first point of the line by 0.0000001 of it's length.
* @return
*/
static Line2D tweakLine(Line2D l) {
Line2D ql = new Line2D.Double(
l.getX1() + 0.0000001*(l.getX2() - l.getX1()),
l.getY1() + 0.0000001*(l.getY2() - l.getY1()),
l.getX2() - 0.0000001*(l.getX2() - l.getX1()),
l.getY2() - 0.0000001*(l.getY2() - l.getY1()));
return ql;
}
public class ShamosHoeyAlgorithm {
public static boolean ShamosHoeyAlgorithm(Collection lines) {
List events = new ArrayList(lines.size() * 2);
for (Line2D li : lines) {
if (li.getX1() li.getX2()) {
Line2D l = new Line2D.Double(li.getP2(), li.getP1());
events.add(new AlgEvent(l, true));
events.add(new AlgEvent(l, false));
} else {
if (li.getY1() li.getY2()) {
Line2D l = n
Solution
The question of whether your code is performant enough is something your profiler can better answer for you. But from looking through the code above I notice quite a bit of duplication along with some rather deeply nested if's which you should try to refactor. Take this for example:
The two statements
Your line compare method here:
can be shorted by taking advantage of logical short-circuit and the ternary operator. So something like this might be easier to read:
You can apply the same idea to AlgEvtComparator's compare method. One other thing I noticed in your line compare method, the checks' aren't exactly symmetrical. You have o1.Y1 comparing to o2.Y2 while all the others are checking Y1 to Y1 or Y2 to Y2. Was that really intended? I think this deserves a comment.
I'm guessing Line2D is a class you have defined somewhere. You might want to see if you're abstracting its usage enough or if an 'in-between' class is needed. The following code looks like it's leaking stuffing behind Line2D's interface:
if (li.getX1() li.getX2()) {
Line2D l = new Line2D.Double(li.getP2(), li.getP1());
events.add(new AlgEvent(l, true));
events.add(new AlgEvent(l, false));
} else {
if (li.getY1() li.getY2()) {
Line2D l = new Line2D.Double(li.getP2(), li.getP1());
events.add(new AlgEvent(l, true));
events.add(new AlgEvent(l, false));
} else
// ...The two statements
events.add(new AlgEvent(l, true)); and events.add(new AlgEvent(l, false)); are being repeated 4 times here!Your line compare method here:
static class LineComparator implements Comparator {
public int compare(Line2D o1, Line2D o2) {
if (o1.getY1() o2.getY2()) {
// ...
}can be shorted by taking advantage of logical short-circuit and the ternary operator. So something like this might be easier to read:
public int compare(Line2D o1, Line2D o2)
{
/* I'm not too familiar with java but can
the equals method be used here to check
if the lines are equal?
*/
// if( o1.equals(o2) ) return 0;
return (o1.getY1() o2.getY2() ||
o1.getY2() > o2.getY2()) ? 1 : 0;
}You can apply the same idea to AlgEvtComparator's compare method. One other thing I noticed in your line compare method, the checks' aren't exactly symmetrical. You have o1.Y1 comparing to o2.Y2 while all the others are checking Y1 to Y1 or Y2 to Y2. Was that really intended? I think this deserves a comment.
I'm guessing Line2D is a class you have defined somewhere. You might want to see if you're abstracting its usage enough or if an 'in-between' class is needed. The following code looks like it's leaking stuffing behind Line2D's interface:
if (estPath.size() != 0) {
Point2D pp = estPath.get(estPath.size() - 1).getP2();
estPath.add(new Line2D.Double(pp, new Point2D.Double(coords[0],coords[1])));
} else {
estPath.add(new Line2D.Double(new Point2D.Double(), new Point2D.Double(coords[0],coords[1])));
}Code Snippets
if (li.getX1() < li.getX2()) {
Line2D l = new Line2D.Double(li.getP1(), li.getP2());
events.add(new AlgEvent(l, true));
events.add(new AlgEvent(l, false));
} else if (li.getX1() > li.getX2()) {
Line2D l = new Line2D.Double(li.getP2(), li.getP1());
events.add(new AlgEvent(l, true));
events.add(new AlgEvent(l, false));
} else {
if (li.getY1() < li.getY2()) {
Line2D l = new Line2D.Double(li.getP1(), li.getP2());
events.add(new AlgEvent(l, true));
events.add(new AlgEvent(l, false));
} else if (li.getY1() > li.getY2()) {
Line2D l = new Line2D.Double(li.getP2(), li.getP1());
events.add(new AlgEvent(l, true));
events.add(new AlgEvent(l, false));
} else
// ...static class LineComparator implements Comparator<Line2D> {
public int compare(Line2D o1, Line2D o2) {
if (o1.getY1() < o2.getY1()) {
return -1;
} else if (o1.getY1() > o2.getY2()) {
// ...
}public int compare(Line2D o1, Line2D o2)
{
/* I'm not too familiar with java but can
the equals method be used here to check
if the lines are equal?
*/
// if( o1.equals(o2) ) return 0;
return (o1.getY1() < o2.getY1() ||
o1.getY2() < o2.getY2()) ? -1 :
(o1.getY1() > o2.getY2() ||
o1.getY2() > o2.getY2()) ? 1 : 0;
}if (estPath.size() != 0) {
Point2D pp = estPath.get(estPath.size() - 1).getP2();
estPath.add(new Line2D.Double(pp, new Point2D.Double(coords[0],coords[1])));
} else {
estPath.add(new Line2D.Double(new Point2D.Double(), new Point2D.Double(coords[0],coords[1])));
}Context
StackExchange Code Review Q#69, answer score: 11
Revisions (0)
No revisions yet.