Search for everything at a tile in Minesweeper

Question

I'm writing a Minesweeper clone. I needed a function that returns everything located at a certain tile, and ended up with an atrocious mess. I'd like help making it cleaner and more idiomatic.

The main state of the board is represented by a Field record:

(defrecord Field [dimensions ; Pair: [width height] in cells
                  flags ; Set
                  bombs ; Set
                  hints ; Map: Position -> hint value
                  uncovered-cells]) ; Set

Cell contents are tracked using sets for flags and bombs (since I only care about whether or not they're there), and a map for the hints (since I need to track the value of the hint as well). The keys for the sets/map are [x y] pairs representing a coordinate position, while the value for the hint map is a number between 1 and 8.

As mentioned above, I needed a function that returns everything at a certain coordinate. I ended up with this mess:

(defn- get-by-key [field x y key]
  ((key field) [x y]))

(defn all-at
  "Returns a set of everything at the position.
  Possible contents are :bomb, :flag, or a number representing a hint."
  [field x y]
  (let [g #(get-by-key field x y %)
        flag? (if (g :flags) :flag nil)
        bomb? (if (g :bombs) :bomb nil)
        hint? (g :hints)
        hint'? (if hint? hint? nil)]
    (into #{} ; So it can be easily searched for membership
          (filter #(not (nil? %))
                  [flag? bomb? hint'?]))))

I'm thoroughly unhappy with this though. It's giant for something that superficially seemed like a simple job, and has a lot of copied parts. The need for the filter also seems wrong, especially since I immediately force it into a set, so all laziness is gone.

I need it to check :flags, :bombs, and :hints to see if they contain the given coordinate, and if they do, return a set of everything at the position.

How can I write this better? Any suggestions here would be appreciated.

Answer 1

A few things:

I'd suggest that get-by-key can more clearly be defined as an inline lambda using get-in instead of implicitly calling the keys. You can also inline the construction of the flag?-bomb?-hint? vector to cut down on definitions.

I wouldn't worry about the filter immediately going into a set. Sets do one thing and filter does another; the laziness doesn't need to come into play.

Also, this:

(if (g :flags) :flag nil)

is more idiomatic as

(when (g :flags) :flag)

and

#(not (nil? %))

is

some?

---

Here is a shorter version of your function, taking those into account and not declaring temporary bindings for flag?, bomb?, hint?. I think in this fairly simple scope, that's fine.

(defn all-at
  "Returns a set of everything at the position.
  Possible contents are :bomb, :flag, or a number representing a hint."
  [field x y]
  (let [get-at #(get-in field [% [x y]])]
    (->> [(when (get-at :flags) :flag)
          (when (get-at :bombs) :bomb)
          (get-at :hints)]
         (filter some?)
         (into #{}))))