HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Refactor code to find four co-linear points

Submitted by: @import:stackexchange-codereview··
0
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 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

if (first.equals(next)) {
    // do stuff
}


Instead write

if (!first.equals(next)) {
    continue;
}
// do stuff


This 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 stuff
SortedSet<Point> line = new TreeSet<Point>();

Context

StackExchange Code Review Q#35693, answer score: 5

Revisions (0)

No revisions yet.