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

A Quadtree implementation with Colliders

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
withimplementationquadtreecolliders

Problem

I have created a QuadTree in Java and would like a general code review or suggestions.

```
import java.util.ArrayList;

public class QuadTree
{
private QuadTree root, parent, northEast, southWest, northWest, southEast;
Point center;
Double halfDimension;
Double northBound, eastBound, southBound, westBound;
ArrayList Objects;

public QuadTree(Point center, Double halfDimension, QuadTree parent)
{
this.center = center;
this.halfDimension = halfDimension;
northBound = center.y + halfDimension;
southBound = center.y - halfDimension;
eastBound = center.x + halfDimension;
westBound = center.x - halfDimension;
this.parent = parent;
northEast = null;
southWest = null;
northWest = null;
southEast = null;
Objects = new ArrayList();
if (this.parent == null)
{
root = this;
}
else
{
root = parent.root;
}
}

public ArrayList getColliders(Circle area)
{

ArrayList results = new ArrayList();
for (iCollider c : Objects)
{
if (Point.getDistanceBetweenPoints(area.center, c.getBody().center) center.x)
{
if (area.center.y + area.radius > center.y && northEast != null)
{
results.addAll(northEast.getColliders(area));
}
if (area.center.y - area.radius center.y && northWest != null)
{
results.addAll(northWest.getColliders(area));
}
if (area.center.y - area.radius getChildren()
{
ArrayList children = new ArrayList();

for (Quadrant q : Quadrant.values())
{
if (this.getSubQuadTree(q) != null)
{
children.add(this.getSubQuadTree(q));
}
}
return children;
}

public ArrayList getAllColliders()
{
ArrayList results = ne

Solution

This code is difficult to review for many reasons:

  • It's not following standard formatting



  • It's not following standard naming, for example: iCollider, Objects



  • There are some unknown external elements, for example: ColliderType, SFAServer



  • Almost half of the methods are unused (not referenced within your post)



It would be better to post your best attempt, something you already cleaned up and criticized yourself.

Make fields private

As much as possible, make all fields private, and add some getters and setters when necessary

Make fields immutable

Classes with immutable fields are great. Immutable things are the least troublesome elements, they are robust, and inherently thread-safe. For example all these fields could be declared final:

  • x and y in Point



  • center and radius in Circle



  • center, halfDimension, northBound, ..., objects in QuadTree



Use interface types

Use interfaces in variable and method declarations as much as possible. Don't do this:

public ArrayList getColliders(Circle area) {
    ArrayList results = new ArrayList();


Do like this instead:

public List getColliders(Circle area) {
    List results = new ArrayList();


About Quadrant.value

To answer your question in this code:

enum Quadrant {
    NORTHEAST(1), NORTHWEST(2), SOUTHWEST(3), SOUTHEAST(4);
    int value;

    private Quadrant(int value) {
        // not sure what the point of this is
        this.value = value;
    }
}


If you're not sure what the point is, then delete it and see what breaks. In any case, the code you posted is not using it.

Pointless comments

Don't comment like this:

if (p.y > center.y) {
    return Quadrant.NORTHEAST;
    // return northEast;
} else {
    return Quadrant.NORTHWEST;
    // return northWest;
}


Actually, it's not good to comment in general.

Many more...

IDEs usually warn you about things that are not so good. I'm using the community edition of IntelliJ and your code is full of many more warnings that are easy to fix. I'm sure Eclipse and others would give you many warnings too. In Eclipse I would especially recommend the FindBugs plugin. By following the advices of these tools, you can learn a lot and naturally avoid the most common mistakes.

Code Snippets

public ArrayList<iCollider> getColliders(Circle area) {
    ArrayList<iCollider> results = new ArrayList<iCollider>();
public List<iCollider> getColliders(Circle area) {
    List<iCollider> results = new ArrayList<iCollider>();
enum Quadrant {
    NORTHEAST(1), NORTHWEST(2), SOUTHWEST(3), SOUTHEAST(4);
    int value;

    private Quadrant(int value) {
        // not sure what the point of this is
        this.value = value;
    }
}
if (p.y > center.y) {
    return Quadrant.NORTHEAST;
    // return northEast;
} else {
    return Quadrant.NORTHWEST;
    // return northWest;
}

Context

StackExchange Code Review Q#58283, answer score: 2

Revisions (0)

No revisions yet.