One of the hardest things about learning a new language is figuring out the idiomatic way of doing something in that language. That’s currently where I’m at while taking a stab at ClojureScript for a project that I’m working on.

The problem:

Imagine a piece of grid paper. Your job is to draw a map on that grid paper. Also imagine that when you place a wall, or rather, carve out a room from a solid wall, you actually carve out a section from each of the 8 surrounding squares as well.

Here is a visual representation of what I’m talking about:

There are basically a set of tiles stored in an array:

[ a b c
  d e f
  g h i ]

Each tile is composed of, what is essentially, a 9-bit integer where 0 is :floor and 1 is :wall.

The algorithm for carving out a wall is fairly simple:

1. Each corner has a bit-mask representation
2. The current value of the tile is bit-anded with the tile mask
3. The result is stored back in the array

For example, to set the value for the top-left corner, it might look something like this:

let [tl (bit-and (get tiles (index-offset row-1 col-1)) tile-top-left)
     ...
new-tiles (-> tiles
              (assoc (index-offset row-1 col-1) tl)
...

That’s not super great. It also doesn’t feel very “clojurey” to me, even though I don’t have a good idea of what that really means yet.

Aside: I really enjoy being able to use characters like ? and - in names of things… so much clearer!

Now… here’s the thing. This function actually works. So really, I could stop here and things would be fine. However, this code has a bunch of debt in it as there are essentially over 20 lines of code that have a lot of duplication in it.

I’d like to clean that up.

Consolidating

The entire code chunk:

(defn place-floor
  [tiles row col]
  (let [row-1 (dec row)
        row+1 (inc row)
        col-1 (dec col)
        col+1 (inc col)
        tl (bit-and (get tiles (index-offset row-1 col-1)) tile-top-left)
        tc (bit-and (get tiles (index-offset row-1 col)) tile-top-center)
        tr (bit-and (get tiles (index-offset row-1 col+1)) tile-top-right)
        lc (bit-and (get tiles (index-offset row col-1)) tile-left-center)
        rc (bit-and (get tiles (index-offset row col+1)) tile-right-center)
        bl (bit-and (get tiles (index-offset row+1 col-1)) tile-bottom-left)
        bc (bit-and (get tiles (index-offset row+1 col)) tile-bottom-center)
        br (bit-and (get tiles (index-offset row+1 col+1)) tile-bottom-right)

        new-tiles (-> tiles
                      (assoc (index-offset row-1 col-1) tl)
                      (assoc (index-offset row-1 col) tc)
                      (assoc (index-offset row-1 col+1) tr)
                      (assoc (index-offset row col-1) lc)
                      (assoc (index-offset row col) floor-tile)
                      (assoc (index-offset row col+1) rc)
                      (assoc (index-offset row+1 col+1) br)
                      (assoc (index-offset row+1 col) bc)
                      (assoc (index-offset row+1 col-1) bl))]
    (rf/dispatch-sync [:update-map new-tiles])))

There are a few things to note here:

1. Each call to assoc is going to create a new copy of the tiles for us: not great.
2. The index values have already been calculated once before in the let structure above.
3. The code is a bit hard to grok.

Removing Duplication

The first step I want to do is remove the index-offset calculations. To do that, I’m going to create a new function update-tile.

(defn update-tile
  [tiles row col f arg]
  (let [n (index-offset row col)]
    (assoc tiles n (f (get tiles n) arg))))

This is a function that takes in the vector of tiles, the row and column, a function f, and the argument to pass to that function. The first three arguments are pretty clear, but what is f? Well, f is the operation to do on the given tile: namely bit-and.

Why do I have this? Well… for placing a wall tile, I need to perform a bit-or operation and I’d like to generalize this function as it will apply to both scenarios equally well.

The resulting code now looks like this:

(defn place-floor
  [tiles row col]
  (let [row-1 (dec row)
        row+1 (inc row)
        col-1 (dec col)
        col+1 (inc col)

        new-tiles (-> tiles
                      (update-tile row-1 col-1 bit-and tile-top-left)
                      (update-tile row-1 col bit-and tile-top-center)
                      (update-tile row-1 col+1 bit-and tile-top-right)
                      (update-tile row col-1 bit-and tile-left-center)
                      (update-tile row col bit-and floor-tile)
                      (update-tile row col+1 bit-and tile-right-center)
                      (update-tile row+1 col+1 bit-and tile-bottom-right)
                      (update-tile row+1 col bit-and tile-bottom-center)
                      (update-tile row+1 col-1 bit-and tile-bottom-left))]
    
    (rf/dispatch-sync [:update-map new-tiles])))

Already this is so much better! However, I still have some duplication: the bit-and parameter. For that, I’ll simply make an update-tile-and function.

(defn place-floor
  [tiles row col]
  (let [row-1 (dec row)
        row+1 (inc row)
        col-1 (dec col)
        col+1 (inc col)

        new-tiles (-> tiles
                      (update-tile-and row-1 col-1 tile-top-left)
                      (update-tile-and row-1 col tile-top-center)
                      (update-tile-and row-1 col+1 tile-top-right)
                      (update-tile-and row col-1 tile-left-center)
                      (update-tile-and row col floor-tile)
                      (update-tile-and row col+1 tile-right-center)
                      (update-tile-and row+1 col+1 tile-bottom-right)
                      (update-tile-and row+1 col tile-bottom-center)
                      (update-tile-and row+1 col-1 tile-bottom-left))]

    (rf/dispatch-sync [:update-map new-tiles])))

Good deal!

Destructuring

There’s nothing in particular that is wrong with this:

  (let [row-1 (dec row)
        row+1 (inc row)
        col-1 (dec col)
        col+1 (inc col)

However, it can be written a little more concisely and in a way that I think is a little more clear:

  (let [[row-1 row+1] [(dec row) (inc row)]
        [col-1 col+1] [(dec col) (inc col)]

This is called “destructuring”. You can read more about it here: https://clojure.org/guides/destructuring.

In the End

The final code looks like this:

(defn place-floor
  [tiles row col]
  (let [[row-1 row+1] [(dec row) (inc row)]
        [col-1 col+1] [(dec col) (inc col)]
        new-tiles (-> tiles
                      (update-tile-and row-1 col-1 tile-top-left)
                      (update-tile-and row-1 col tile-top-center)
                      (update-tile-and row-1 col+1 tile-top-right)
                      (update-tile-and row col-1 tile-left-center)
                      (update-tile-and row col floor-tile)
                      (update-tile-and row col+1 tile-right-center)
                      (update-tile-and row+1 col+1 tile-bottom-right)
                      (update-tile-and row+1 col tile-bottom-center)
                      (update-tile-and row+1 col-1 tile-bottom-left))]

    (rf/dispatch-sync [:update-map new-tiles])))

I still don’t know if this the “idiomatic way” of writing ClojureScript. Really, it looks a heck of a lot like how I’d write this in C++ or Swift. Sure, the syntax is a bit different, but it’s essentially imperative flow. I don’t know if that’s “right”.

However, what I do know is:

1. The code above gets rid of almost all of the duplication that I had before.
2. The code more readable.
3. The code is more maintainable.

I call that a win.