3
\$\begingroup\$

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.

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

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 #{}))))
\$\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.