patternjavaMinor
Refactor code to find four co-linear points
Viewed 0 times
linearpointsfourfindcoderefactor
Problem
I am relatively new to Java, but have been reading up on how to improve the quality of my code.
I am writing a system where I take in a series of points from a file with x and y co-ordinates and by checking slopes relative to each other, am calculating and drawing a line if there is four or more co-linear points on the line. The code works and is doing what is required, but I would like to tidy up this method by reducing the
I think it's messy, but not sure how to rectify it.
```
public static void checkForLines(Double[] arrayOfSlopes, Point[] tempArray)
{
int count = 0; // Keeps track of number of duplicates
int i = 1;
while (i line = new TreeSet();
//Create a set to store points from this line
line.add(tempArray[0]);
line.add(tempArray[i - 1]);
line.add(tempArray[i]);
line.add(tempArray[i + 1]);
//Store four found points
int j = i + 1;
while (j + 1 < arrayOfSlopes.length - 1 && (arrayOfSlopes[j].equals(arrayOfSlopes[j + 1])))
{
line.add(tempArray[j + 1]);
//Store any other further duplicates if exist
j++;
}
if (arrayOfSlopes[j].isInfinite() && arrayOfSlopes[arrayOfSlopes.length - 1].isInfinite() ||
arrayOfSlopes[arrayOfSlopes.length - 1].equals(first))
{
line.add(tempArray[tempArray.length - 1]);
I am writing a system where I take in a series of points from a file with x and y co-ordinates and by checking slopes relative to each other, am calculating and drawing a line if there is four or more co-linear points on the line. The code works and is doing what is required, but I would like to tidy up this method by reducing the
if statements, but am not sure on how to, or if it is even necessary. The parameters being passed in is an array of doubles, which are slopes relative to an origin. I am iterating this array to look for 3 or more equal slopes in a row which would give a line and adding the corresponding points and any further equal points to a set. Once a line has been found, the rest of the slopes in array are checked in similar fashion. Any advice would be appreciated.I think it's messy, but not sure how to rectify it.
```
public static void checkForLines(Double[] arrayOfSlopes, Point[] tempArray)
{
int count = 0; // Keeps track of number of duplicates
int i = 1;
while (i line = new TreeSet();
//Create a set to store points from this line
line.add(tempArray[0]);
line.add(tempArray[i - 1]);
line.add(tempArray[i]);
line.add(tempArray[i + 1]);
//Store four found points
int j = i + 1;
while (j + 1 < arrayOfSlopes.length - 1 && (arrayOfSlopes[j].equals(arrayOfSlopes[j + 1])))
{
line.add(tempArray[j + 1]);
//Store any other further duplicates if exist
j++;
}
if (arrayOfSlopes[j].isInfinite() && arrayOfSlopes[arrayOfSlopes.length - 1].isInfinite() ||
arrayOfSlopes[arrayOfSlopes.length - 1].equals(first))
{
line.add(tempArray[tempArray.length - 1]);
Solution
The easiest way to flatten this kind of a structure is to reverse the sense of the if. That is, where you have
Instead write
This only works where you don't have an
From there I would start seeing where I could extract methods, especially but not exclusively where there is duplication in the code. The block of code beginning with
looks like a good candidate for this.
if (first.equals(next)) {
// do stuff
}Instead write
if (!first.equals(next)) {
continue;
}
// do stuffThis only works where you don't have an
else clause (so unfortunately I chose a bad example), but it can work wonders in bringing your code closer to the left margin.From there I would start seeing where I could extract methods, especially but not exclusively where there is duplication in the code. The block of code beginning with
SortedSet line = new TreeSet();looks like a good candidate for this.
Code Snippets
if (first.equals(next)) {
// do stuff
}if (!first.equals(next)) {
continue;
}
// do stuffSortedSet<Point> line = new TreeSet<Point>();Context
StackExchange Code Review Q#35693, answer score: 5
Revisions (0)
No revisions yet.