1
\$\begingroup\$

I'm new to Clojure and decided to read the book, "Mazes for Programmers". My first task, without reading any of the book, was to think of how I would create a maze. This is my first result and I'd like good, honest and even scathing critiques:

(defrecord Cell [visited? valid-dirs open-dirs])
(defrecord Maze [cells])

(defn create-cell
      "Creates an unvisited cell with no valid directions."
      ([] (Cell. false #{} #{}))
      ([valid-dirs] (Cell. false valid-dirs #{})))

(defn create-corner-cell [corner]
      "Creates a corner cell which has two valid directions."
      (case corner
            (:top-left) (create-cell #{:east :south})
            (:top-right) (create-cell #{:west :south})
            (:bottom-left) (create-cell #{:east :north})
            (:bottom-right) (create-cell #{:west :north})))

(defn create-wall-cell [side]
      "Creates a wall cell which has three valid directions."
      (case side
            (:top) (create-cell #{:west :south :east})
            (:left) (create-cell #{:north :south :east})
            (:bottom) (create-cell #{:west :north :east})
            (:right) (create-cell #{:north :south :west})))

(defn create-unbound-cell []
      "Creates a cell that has no invalid directions."
      (create-cell #{:north :east :south :west}))

(defn visited? [cell]
      "Determines whether or not a cell has been visisted."
      (true? (:visited? cell)))

(defn corner-cell? [index size]
  "Determines whether the index is a corner cell of #{top-left top-right bottom-left bottom-right} in a grid
  of size*size dimensions. Returns both whether or not the index is a corner cell, but
  also which corner of the grid it's on."
      (assert (not= 0 size))
      (cond
            (= index 0) [true :top-left]
            (= index (dec size)) [true :top-right]
            (= index (- (* size size) size)) [true :bottom-left]
            (= index (dec (* size size))) [true :bottom-right]
            :else [false]))

(defn wall-cell? [index size]
  "Determines whether the index is a wall cell of #{top left right bottom} in a grid
   of size*size dimensions. Returns both whether or not the index is a wall cell, but
   also which side of the grid it's on."
      (assert (not= 0 size))
  (let [contains-index (partial seq-contains index)
        top-row (range 0 size)
        bottom-row (range (- (* size size) size) (* size size))
        left-col (take-while (partial > (* size size)) (iterate #(+ % size) 0))
        right-col (take-while (partial > (* size size)) (iterate #(+ % size) (dec size)))]
    (cond
      (contains-index top-row) [true :top]
      (contains-index bottom-row) [true :bottom]
      (contains-index left-col) [true :left]
      (contains-index right-col) [true :right])))

(def m-corner-cell? (memoize corner-cell?))
(def m-wall-cell? (memoize wall-cell?))

(defn create-maze-cell [index size]
      "Creates a cell depending on its location in the maze."
      (let [[is-corner-cell corner] (m-corner-cell? index size)
            [is-wall-cell side] (m-wall-cell? index size)]
            (cond
                  (true? is-corner-cell) (create-corner-cell corner)
                  (true? is-wall-cell) (create-wall-cell side)
                  :else (create-unbound-cell))))

(defn create-maze [size]
      "Create a maze of size*size dimensions."
      (if (= 1 size) (create-cell)
        (let [total-cells (* size size)]
              (map-indexed
                    (fn [idx _] (create-maze-cell idx size))
                    (range total-cells)))))
\$\endgroup\$
2
\$\begingroup\$

Docstrings go BEFORE the arglist

(defn my-function 
  "with aweseome documentation"
  [all the args]
  :insert-code-here)

Visited really shouldn't be a property of a cell. Instead of mutating your cell, you should maintain a set of visited nodes. Maybe you want to run two different path finding algorithms over the same data?

Your case corners are a little strange. Whenever you see this, you should probably think map:

(def corner-cells
  {:top-left #{:east :south}
   :top-right #{:west :south}
   :bottom-left #{:east :north}
   :bottom-right #{:west :north}})

(defn create-corner-cell 
  "Creates a corner cell which has two valid directions."
  [corner]
  (create-cell (corner-cells corner)))

Do you really need :top-left and all those combinations? Consider instead:

(def open-block #{:north :east :south :west})
(def wall-for-side
  {:top :north
   :bottom :south
   :left :west
   :right :east})

(defn add-walls [block & sides]
  (clojure.set/difference block
                          (map wall-for-side sides)))


mazer.core> (add-walls open-block :top)
#{:west :south :east}
mazer.core> (add-walls open-block :top :left)
#{:south :east}
mazer.core> (add-walls open-block :bottom :right)
#{:west :north}

A function like corner-cell? doesn't need to return a list like that. Simply return nil for false and the direction for true. You can use (if-let [dir (corner-cell? ...)] :something-with-dir :something-without-dir) to differentiate. And, since this isn't strictly a predicate, you really shouldn't name it with a question mark. Maybe corner-cell-type.

I wouldn't use memoization here. You almost never need to memoize. If you REALLY think you need to cache this, that's the kind of information that probably would go on the cell. But I think you should just convert the index to an x/y (math is fast) and compare that.

\$\endgroup\$
1
\$\begingroup\$

I'd say looks very good. Some small notes:

  • The indentation is off, although it looks like it could be related to tabs/spaces?
  • Using true? is probably not necessary, if you return a true value it doesn't really matter if it's exactly true, or some other true value.
  • wall-cell? can be split into more functions since some code is duplicated.
  • The create-corner/wall-cell functions can be a bit simplified if the order of applications is switched around a bit. They also don't handle errors.

(I couldn't find seq-contains anywhere, so I defined it in the snippet below.) Updated looks like this:

(defn seq-contains [elm seq]
  (some #(= elm %) seq))

(defrecord Cell [visited? valid-dirs open-dirs])
(defrecord Maze [cells])

(defn create-cell
  "Creates an unvisited cell with no valid directions."
  ([] (Cell. false #{} #{}))
  ([valid-dirs] (Cell. false valid-dirs #{})))

(defn create-corner-cell [corner]
  "Creates a corner cell which has two valid directions."
  (create-cell
   (case corner
     :top-left #{:east :south}
     :top-right #{:west :south}
     :bottom-left #{:east :north}
     :bottom-right #{:west :north})))

(defn create-wall-cell [side]
  "Creates a wall cell which has three valid directions."
  (create-cell
   (case side
     :top #{:west :south :east}
     :left #{:north :south :east}
     :bottom #{:west :north :east}
     :right #{:north :south :west})))

(defn create-unbound-cell []
  "Creates a cell that has no invalid directions."
  (create-cell #{:north :east :south :west}))

(defn visited? [cell]
  "Determines whether or not a cell has been visisted."
  (true? (:visited? cell)))

(defn corner-cell? [index size]
  "Determines whether the index is a corner cell of #{top-left top-right bottom-left bottom-right} in a grid
  of size*size dimensions. Returns both whether or not the index is a corner cell, but
  also which corner of the grid it's on."
  (assert (not= 0 size))
  (let [squared-size (* size size)]
    (cond
     (= index 0) [true :top-left]
     (= index (dec size)) [true :top-right]
     (= index (- squared-size size)) [true :bottom-left]
     (= index (dec squared-size)) [true :bottom-right]
     :else [false])))

(defn create-column (index)
  (take-while (partial > size-squared) (iterate #(+ % size) index)))

(defn wall-cell? [index size]
  "Determines whether the index is a wall cell of #{top left right bottom} in a grid
   of size*size dimensions. Returns both whether or not the index is a wall cell, but
   also which side of the grid it's on."
  (assert (not= 0 size))
  (let [contains-index (partial seq-contains index)
        top-row (range 0 size)
        size-squared (* size size)
        bottom-row (range (- size-squared size) size-squared)
        left-col (create-column 0)
        right-col (create-column (dec size))]
    (cond
     (contains-index top-row) [true :top]
     (contains-index bottom-row) [true :bottom]
     (contains-index left-col) [true :left]
     (contains-index right-col) [true :right])))

(def m-corner-cell? (memoize corner-cell?))
(def m-wall-cell? (memoize wall-cell?))

(defn create-maze-cell [index size]
  "Creates a cell depending on its location in the maze."
  (let [[is-corner-cell corner] (m-corner-cell? index size)
        [is-wall-cell side] (m-wall-cell? index size)]
    (cond
     is-corner-cell (create-corner-cell corner)
     is-wall-cell (create-wall-cell side)
     :else (create-unbound-cell))))

(defn create-maze [size]
  "Create a maze of size*size dimensions."
  (if (= 1 size)
    (create-cell)
    (let [total-cells (* size size)]
      (map-indexed
       (fn [idx _] (create-maze-cell idx size))
       (range total-cells)))))
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.