Travelling Salesman Problem with visualisation in Java

For practicing purposes, I challenged myself to write a program that solves the TSP and visualises the results step by step.

As for now, my program uses a simple nearest neighbour algorithm. I want my program to be flexible, so when I add a new algorithm, it will be able to visualise the results, too, without messing with the logic of display.

One of the problems I encountered was displaying the solution step by step. I solved it by creating multiple partial solutions, storing them, and displaying one after another. I feel like it can be done better, but I am not really good in graphics, so I hope to get some clues here.

The Point class represents a city:

class Point {
    private double x;
    private double y;

    public double getX() {
        return x;
    }
    public double getY() {
        return y;
    }

    public Point(double x, double y) {
        this.x = x;
        this.y = y;
    }

    public Point(){
        Random r = new Random();
        x=r.nextInt(1000);
        y=r.nextInt(650);
    }

    public double calculateDistanceToPoint(Point p) {
        double dist = Math.sqrt(Math.pow(this.x-p.x, 2) + Math.pow(this.y-p.y, 2));
        return round(dist,2);
    }

    private static double round(double value, int places) {
        if (places < 0) throw new IllegalArgumentException();

        BigDecimal bd = new BigDecimal(value);
        bd = bd.setScale(places, RoundingMode.HALF_UP);
        return bd.doubleValue();
    }
}

Then, the Solver class, which is doing calculations:

```
class Solver {
//list of all points to visit
private static ArrayList points = new ArrayList<>();

//adjacency matrix
private ArrayList> adjMatrix = new ArrayList<>();

//found solution
private static ArrayList solution = new ArrayList<>();

//visited points
private ArrayList visitedPoints = new ArrayList<>();

//used for visualisation
private static Solution finalSolution = new Solution();

public void clear()

I feel like I'm using too much static fields and methods- but on the other hand, would it be better to create instances of, for example, Solver and then have non-static methods to get the solutions?

Sure! About the only sane reason for using static is a utility class like java.lang.Math. A static class with mutable state like yours is a sin. Just imagine two threads trying to solve a TSP at the same time.
Point

class Point {
    private double x;
    private double y;

I see that your Point is immutable. That's a good decision, but then also declare x and y as final. This makes your class thread-safe and makes the immutability obvious.

public Point(){
        Random r = new Random();
        x=r.nextInt(1000);
        y=r.nextInt(650);
    }

Creating a Random instance for a single use doesn't feel right. With

public Point(Random random) {
        x = random.nextInt(SCREEN_WIDTH);
        y = random.nextInt(SCREEN_HEIGHT);
    }

you allow the user to make their random TSP reproducible (by seeding the random). Note also the spacing and constants.

public double calculateDistanceToPoint(Point p) {
        double dist = Math.sqrt(Math.pow(this.x-p.x, 2) + Math.pow(this.y-p.y, 2));
        return round(dist,2);
    }

Most of the time, any rounding outside of presentation is wrong.

private static double round(double value, int places) {
    if (places < 0) throw new IllegalArgumentException();

    BigDecimal bd = new BigDecimal(value);
    bd = bd.setScale(places, RoundingMode.HALF_UP);
    return bd.doubleValue();
}

This looks rather inefficient, but that's fine for your case. I'd simply go for

private static double roundToTwoPlaces(double value) {
    return 0.01 * Math.round(100 * value);
}

as it's much simpler and good enough. Still, I can't see what you gain by this rounding...
Solver

class Solver {
    //list of all points to visit
    private static ArrayList points = new ArrayList<>();

Either use proper javadoc or leave it out. I guess everyone knowing TSP will thing that the points are to be visited. Maybe you should call it pointsToBeVisited.. probably no, too long and too small added value. Maybe cities? Or just leave it as is.

But always use List instead of ArrayList in declarations. Here, it rather doesn't matter, but in a parameter list it would.

//adjacency matrix
private ArrayList> adjMatrix = new ArrayList<>();

Again, either adjMatrix is clear enough (to me it is) or name it adjacencyMatrix.

Consider using an array here as you never need it to grow. An double[][] is quite a bit faster as you need no autoboxing.

//used for visualisation
    private static Solution finalSolution = new Solution();

This should ideally be not there. Maybe adding a callback (e.g., a Callable) to be called after each step would help.

public static ArrayList getPoints() {
        return Solver.points;
    }

By returning the internal list, you allow everyone who gets it to manipulate it later. So everything may happen and you've lost any encapsulation. Either return a copy, or provide getPoint(int index), or just leave it out.

public void fillAdjacencyMatrix() {
        int iter_x;
        int iter_y;
        for (iter_x = 0; iter_x  temp = new ArrayList<>();
            for (iter_y = 0; iter_y < points.size(); iter_y++) {

No, please. No iter_x as underscore shouldn't be used except in constant names. No declaration before the loop as the scope should always be minimized. I'd go for

public void fillAdjacencyMatrix() {
        for (int ix = 0; ix  temp = new ArrayList<>();
            for (int iy = 0; iy < points.size(); iy++) {

if I really wanted to make the correspondence between ix/iy and x/y clear.... but there's no x and y there. So use just plain i and j. It caries no information and this is fine as there's no information to carry.

if (iter_x == iter_y) {
                    temp.add(-1.0);

This doesn't feel right. The distance from a point to itself is 0 not -1. Ideally, you should never need such a distance. Leave out this branch if you can.

private int getIndexOfMin(ArrayList arr) {
    Double min = Double.MAX_VALUE;

Don't use Double when you need double.

int index = -2;

Why not -3? ...

SolutionStep ss = new SolutionStep();
        Point p;
        for (Point newPoint : solution) {
            p = new Point(newPoint.getX(), newPoint.getY());

Declare p in the right place, i.e., two lines down. You save one line and do the right thing.

I've got lazy...

StackExchange Code Review Q#94255, answer score: 3