Spiral a matrix of size NxN filled with integers from 1 up to and including n^2

Question

I have written a solution in Clojure for the following question:

When starting from the number 1 and adding three numbers on each row a 3x3 matrix is formed as follows:

1 2 3
4 5 6
7 8 9

Starting from the upper left and spiraling inwards in a clockwise direction the following series is obtained: 1 2 3 6 9 8 7 4 5.

Create a method spiral that finds the spiraling series for a \$n\times n\$ matrix.

I created a long and short solution. They are inspired by the following Stack Overflow answer: ruby - Looping in a spiral outside-in

Version with descriptive names:

(ns spiral.core
  (:require [clojure.core.matrix :as m]))

(defn create-numbers-matrix
  "creates a size x size matrix of numbers from 1 up to and including size."
  [size]
  (let [numbers (range 1 (inc (* size size)))
        partitions (partition size numbers)
        matrix (m/matrix partitions)]
    matrix))

(defn get-first-row [m]
  "gets the first row (index 0) of matrix m."
  (m/get-row m 0))

(defn delete-first-row [m]
  "returns matrix with the first row of the matrix m removed."
  (drop 1 (m/rows m)))

(defn transpose [m]
  "transposes matrix m."
  (m/transpose m))

(defn reverse-rows [m]
  "returns matrix m with rows reversed."
  (m/matrix (reverse m)))

(defn rotate-left [m]
  "rotates the matrix m 90 degrees to the left."
  (-> m
      transpose
      reverse-rows))

(defn rotate-and-chop [m]
  "repeatedly rotates matrix m 90 degrees and cuts off the 
   top till matrix is empty."
  (if (empty? m) nil
      (conj (rotate-and-chop (-> m
                                 delete-first-row
                                 rotate-left))
            (get-first-row m))))

(defn spiral [size]
  "creates list of values in spiral from grid of size size"
  (let [numbers-matrix
        (create-numbers-matrix size)]
    (flatten (rotate-and-chop numbers-matrix))))     

Short version:

(ns spiral.core
  (:require [clojure.core.matrix :as m]))

(defn spiral-one-liner [size]
  (flatten
   ((fn loop [m]
      (if (empty? m) nil
          (conj (loop (m/matrix (reverse (m/transpose (drop 1 (m/rows m))))))
                (m/get-row m 0))))
    (m/matrix (partition size (range 1 (inc (* size size))))))))

The short version is still longer than the Ruby version from the answer (because tail recursions errors appear the loop and recur syntax can, I believe, not be used).

I would be grateful if someone can take a look and provide some suggestions.

Answer 1

First off, the docstring for a function always needs to come before the argument vector. You did this right in create-numbers-matrix, but in the rest of your functions, your docstring will be treated as part of the function body, evaluated, and ignored.

(require '[clojure.repl :refer [doc]])

(doc rotate-and-chop)
;; -------------------------
;; spiral.core/rotate-and-chop
;; ([m])
;;   nil
;=> nil

Some Clojure lint tools like Eastwood will catch this sort of error. Also, as you can see, the doc function indents the first line of the docstring by two spaces, so it's conventional to indent each line of the docstring after the first by two spaces to make everything line up:

(defn rotate-and-chop
  "repeatedly rotates matrix m 90 degrees and cuts off the
  top till matrix is empty."
  [m]
  (if (empty? m) nil
      (conj (rotate-and-chop (-> m
                                 delete-first-row
                                 rotate-left))
            (get-first-row m))))

(doc rotate-and-chop)
;; -------------------------
;; user/rotate-and-chop
;; ([m])
;;   repeatedly rotates matrix m 90 degrees and cuts off the
;;   top till matrix is empty.
;=> nil

For create-numbers-matrix, your current approach with let works fine. You could also avoid having to name each of the intermediate steps by using ->>:

(defn create-numbers-matrix
  "creates a size x size matrix of numbers from 1 up to and including size."
  [size]
  (->> (range 1 (inc (* size size)))
       (partition size)
       m/matrix))

Your get-first-row, delete-first-row, transpose, and reverse-rows functions are equivalent to the already available first, rest, transpose, and reverse functions, respectively, so I would just remove them; they add no value to your code.

It's conventional to indent the "then" branch (and the "else" branch, if present) of an if or when expression by two spaces. So in rotate-and-chop, since your first branch is nil, it would be more idiomatic to write it as (when-not (empty? m) ,,,) or just (when (seq m) ,,,). Also, it might be more readable to put the call to rotate-and-chop into the threading macro:

(defn rotate-and-chop
  "repeatedly rotates matrix m 90 degrees and cuts off the
  top till matrix is empty."
  [m]
  (when (seq m)
    (conj (-> m rest rotate-left rotate-and-chop)
          (first m))))

In spiral, you use flatten to concatenate the elements of the sequence returned by rotate-and-chop into a single sequence. While this does work in your case, I would generally advise against using flatten, because it can lead to unexpected behavior when you don't know what type of data is in the sequences you're concatenating. Usually you just need to flatten the outermost layer, in which case you can simply apply the concat function:

(defn spiral
  "creates list of values in spiral from grid of size size"
  [size]
  (let [numbers-matrix (create-numbers-matrix size)]
    (apply concat (rotate-and-chop numbers-matrix))))

As you noted in your question, though, this is still considerably longer than the Ruby solution. However, it's quite possible to write a short, idiomatic Clojure solution to this problem:

(defn spiral [m]
  (when (seq m) (concat (first m) (-> m rest m/transpose reverse spiral))))

This is basically a direct translation of the Ruby solution into Clojure, except without that ugly modification of the original matrix. When m is empty, I return an empty result (it's common to treat nil as an empty sequence in Clojure). Otherwise, I return the concatenation of the first row of m with the spiral sequence of the reversal of the transposition of the rest of m. The reason I have to call rest is that, in the original Ruby code, shift causes the side effect of removing the first row from the matrix, which I don't want to do here.

You could test this out with a function that generates an example matrix, similar to your create-numbers-matrix function:

(defn matrix [n]
  (partition n (range 1 (inc (* n n)))))

(spiral (matrix 3))
;=> (1 2 3 6 9 8 7 4 5)