Snail matrix in Java - version II

Question

(This post is the continuation of Snail matrix in Java. Also, this post has an continuation.)

As previously, a snail matrix is any square matrix \$M_n = \mathbb{N}^{n \times n}\$ with cell elements from the set \$\{ 1, 2, \ldots, n^2 \}\$ such that they resemble the snail pattern. For example, for \$n = 4\$, \$M_n\$ looks like that:

 1  2  3  4 
12 13 14  5 
11 16 15  6 
10  9  8  7 

This time, I have incorporated an awesome answer by Alexander Ivanchenko, and I ended up with this:

package com.github.coderodde.fun.snailmatrix;

/**
 * This class provides a static method {@link SnailMatrix#ofSize(int) } for 
 * generating snail matrices. A snail matrix is any square matrix with all the
 * cell values {@code 1, 2, ..., n x n} in a snail pattern. For example, for
 * {@code n = 3}, the matrix will look like this:
 * <code>
 * 1 2 3
 * 8 9 4
 * 7 6 5
 * </code>
 */
public class SnailMatrix {
    
    /**
     * The actual matrix data.
     */
    private final int[][] data;
    
    /**
     * Constructs this snail matrix.
     * 
     * @param n the width and height of this matrix. 
     */
    private SnailMatrix(final int n) {
        this.data = new int[n][n];
    }
    
    /**
     * Creates a snail matrix of dimensions {@code n x n}.
     * 
     * @param n the width and height of the resulting matrix.
     * 
     * @return the snail matrix.
     */
    public static SnailMatrix ofSize(final int n) {
        var matrix = new SnailMatrix(n);
        var initializer = new MatrixInitializer(matrix);
        
        initialize(initializer);
        
        return matrix;
    }
    
    /**
     * Initializes the matrix.
     * 
     * @param initializer the intializer object.
     */
    private static void initialize(final MatrixInitializer initializer) {
        int value = 1;
        
        while (initializer.hasNext()) {
            initializer.set(value++);
            initializer.next();
        }
    }
    
    /**
     * Returns the width/height of this matrix. 
     */
    public int size() {
        return data.length;
    }
    
    /**
     * Returns the value from column {@code x} and row {@code y}.
     * 
     * @param x the {@code x}-coordinate of the target cell.
     * @param y the {@code y}-coordinate of the target cell.
     * 
     * @return the value of the target cell.
     */
    public int get(final int x, final int y) {
        return data[y][x];
    }
    
    private void set(final int x, final int y, final int value) {
        data[y][x] = value;
    }
    
    /**
     * This class implements the snail matrix initializer.
     */
    private static final class MatrixInitializer {
        private static final int[][] DIRECTIONS = 
                new int[][] {
                    {1, 0},
                    {0, 1},
                    {-1, 0},
                    {0, -1},
                };
        
        private final SnailMatrix matrix;
        private int direction = 0;
        private int x; 
        private int y;
        
        private MatrixInitializer(final SnailMatrix matrix) {
            this.matrix = matrix;
        }
        
        private void set(int value) {
            matrix.set(x, y, value);
        }
        
        private boolean hasNext() {
            return isValid(x, y) && !isInitialized();
        }
        
        private boolean isValid(final int x, final int y) {
            return 0 <= x && x < matrix.size()
                && 0 <= y && y < matrix.size();
        }
        
        private boolean isInitialized() {
            return isInitialized(x, y);
        }
        
        private boolean isInitialized(final int x, final int y) {
            return matrix.get(x, y) != 0;
        }
        
        private void next() {
            checkDirection();
            x += xDelta();
            y += yDelta();
        }
        
        private void checkDirection() {
            final int nextX = x + xDelta();
            final int nextY = y + yDelta();
            
            if (!isValid(nextX, nextY) || isInitialized(nextX, nextY)) {
                changeDirection();
            }
        }
        
        private int xDelta() {
            return DIRECTIONS[direction][0];
        }
        
        private int yDelta() {
            return DIRECTIONS[direction][1];
        }
        
        private void changeDirection() {
            direction = (direction + 1) % DIRECTIONS.length;
        }
    }
    
    @Override
    public String toString() {
        final StringBuilder sb = new StringBuilder();
        final int capacity = size() * size();
        final int cellLength = Integer.toString(capacity).length();
        final String cellFormat = String.format("%%%dd ", cellLength);
        
        for (int y = 0; y < size(); y++) {
            for (int x = 0; x < size(); x++) {
                sb.append(String.format(cellFormat, get(x, y)));
            }
            
            sb.append("\n");
        }
        
        sb.deleteCharAt(sb.length() - 1); // Delete last newline character.
        return sb.toString();
    }

    public static void main(String[] args) {
        final int n = 4;
        
        System.out.println(SnailMatrix.ofSize(n));
    }
}

Critique request

As always, is there any improvement to be made?

Answer 1

A lot of things are going on inside toString, let's refactor this method.

Avoid hard-coding System properties

Line separator is system-dependent, hence it better to use System.lineSeparator() instead of assuming that it would always be "\n".

Avoid joining Strings manually

Since Java 8 we have a class StringJoiner and overloaded factory method joining() featured by Collectors class.

Leveraging them eliminates the need to control manually the process of generation of a compound String.

Here's how we can refactor toString by making use of built-in Collector joining:

// always use static import with collectors for better readability
import static java.util.stream.Collectors.joining;

class SnailMatrix {

    // ...
    
    private static final String BASE_FORMAT = "%%%dd ";
    
    public String getCellFormat() {
        return BASE_FORMAT.formatted(
            String.valueOf(getCapacity()).length()
        );
    }

    @Override
    public String toString() {
        return Arrays.stream(data)
            .map(this::rowToString)
            .collect(
                joining(System.lineSeparator())
            );
    }

    private String rowToString(int[] row) {
        final var cellFormat = getCellFormat();
        
        return Arrays.stream(row)
            .mapToObj(cellFormat::formatted)
            .collect(joining(" "));
    }
}

Format string holding a template for generating cell format is defined as a static final field.

Helper method rowToString takes advantage of Java 15 String.formatted() (with good old String.format we will be forced to write a lambda instead of a method reference).

And I'll advise to keep the method getCapacity from your previous version because it makes the logic of generating a format string for matrix cells a little bit cleaner.

Method rowToString() can be further improved by eliminating the local variable cellFormat:

private String rowToString(int[] row) {
    return Arrays.stream(row)
        .mapToObj(getCellFormat()::formatted)
        .collect(joining(" "));
}

Note that getCellFormat() is invoked only once when JVM creates an instance of the method reference (not when this instance is being evaluated for each stream element, as one might assume).

Also, I want to point out that it's better to avoid nested streams. One might be tempted to ditch rowToString() method, but my advice is not to do this. By placing a stream which has some complexity to it (not just plain something::stream) inside another stream, you're relinquishing the most powerful weapon of declarative programming - expressiveness.