3
\$\begingroup\$

I've implemented a Game of Life in Clojure and would like to understand what I can do better, especially in terms of idiomatic Clojure (without losing readability/maintainability) of the current version.

This is my first "serious" Clojure program (beyond hello world).

Things that I'm specifically interested in:

  • Anything that's not idiomatic and thus should be changed/improved.
  • Whether P (Point) should be a record ((defrecord Point [x y]) (defn P [x y] (Point. x y))) instead.
(ns com.nelkinda.game-of-life)

(def rules {
    :survival #{2 3}
    :birth #{3}
})

(defn P [x y] {:x x :y y})
(defn neighbors [cell]
    (def neighbors-of-origin #{(P -1 -1) (P -1 0) (P -1 1) (P 0 -1) (P 0 1) (P 1 -1) (P 1 0) (P 1 1)})
    (defn plus [cell1 cell2] {:x (+ (:x cell1) (:x cell2)) :y (+ (:y cell1) (:y cell2))})

    (map #(plus cell %) neighbors-of-origin))

(defn next-generation [life]
    (defn is-alive [cell] (contains? life cell))
    (defn is-dead [cell] (not (is-alive cell)))
    (defn dead-neighbors [cell] (filter #(is-dead %) (neighbors cell)))
    (defn live-neighbors [cell] (filter #(is-alive %) (neighbors cell)))
    (defn count-live-neighbors [cell] (count (live-neighbors cell)))
    (defn born [cell rules] (contains? (:birth rules) (count-live-neighbors cell)))
    (defn survives [cell rules] (contains? (:survival rules) (count-live-neighbors cell)))
    (defn dead-neighbors-of-living-cells [] (mapcat #(dead-neighbors %) life))
    (defn surviving-cells [] (filter #(survives % rules) life))
    (defn born-cells [] (filter #(born % rules) (dead-neighbors-of-living-cells)))

    (into #{} (concat (surviving-cells) (born-cells))))

Test Implementation:

(use 'com.nelkinda.game-of-life)
(use 'clojure.test)

(def universe (atom #{}))

(defn parse
    ([spec] (into #{} (parse (clojure.string/split spec #"") 0 0 '())))
    ([spec x y set] (case (first spec)
        "." (parse (rest spec) (+ x 1) y set)
        "*" (parse (rest spec) (+ x 1) y (cons (P x y) set))
        "\n" (parse (rest spec) 0 (+ y 1) set)
        (nil "") set
        (throw (AssertionError. (str "Wrong input: '" (first spec) "'"))))))

(Given #"the following universe:" [spec]
    (reset! universe (parse spec)))

(Then #"the next generation MUST be:" [spec]
    (assert (= (reset! universe (next-generation @universe)) (parse spec))))

Test Specification:

Feature: Conway's Game of Life

  Rules of Conway's Game of Life
  > The universe of the _Game of Life_ is an infinite, two-dimensional orthogonal grid of square cells.
  > Each cell is in one of two possible states:
  > * Alive aka populated
  > * Dead aka unpopulated
  >
  > Every cell interacts with its eight neighbors.
  > The neighbors are the cells that are horizontally, vertically, or diagonally adjacent.
  > At each step in time, the following transitions occur:
  > 1. Underpopulation: Any live cell with fewer than 2 live neighbors dies.
  > 1. Survival: Any live cell with 2 or 3 live neighbors survives on to the next generation.
  > 1. Overpopulation Any live cell with more than 3 live neighbors dies.
  > 1. Reproduction (birth): Any dead cell with exactly 3 live neighbors becomes a live cell.

  Scenario: Empty universe
    Given the following universe:
    """
    """
    Then the next generation MUST be:
    """
    """

  Scenario: Single cell universe
    Given the following universe:
    """
    *
    """
    Then the next generation MUST be:
    """
    """

  Scenario: Block
    Given the following universe:
    """
    **
    **
    """
    Then the next generation MUST be:
    """
    **
    **
    """

  Scenario: Blinker
    Given the following universe:
    """
    .*
    .*
    .*
    """
    Then the next generation MUST be:
    """
    ***
    """
    Then the next generation MUST be:
    """
    .*
    .*
    .*
    """

  Scenario: Glider
    Given the following universe:
    """
    .*
    ..*
    ***
    """
    Then the next generation MUST be:
    """
    *.*
    .**
    .*
    """
    Then the next generation MUST be:
    """
    ..*
    *.*
    .**
    """
    Then the next generation MUST be:
    """
    .*
    ..**
    .**
    """
    Then the next generation MUST be:
    """
    ..*
    ...*
    .***
    """

Repository URL in case you want to clone it yourself: https://github.com/nelkinda/gameoflife-clojure

\$\endgroup\$
1
  • 1
    \$\begingroup\$ Hello Christian, the usage you made of defn is incorrect. defn usually is used top level because it defines a function in the namesace scope. Each execution of next-generation redefines the bunch of utility functions. If you put them top level they will be defined only once. I suggest you start with that refactoring. \$\endgroup\$
    – Jérôme
    Jan 6 at 20:15

1 Answer 1

2
\$\begingroup\$

First, some formatting nitpicks based on idiomatic Clojure code guidelines:

  • Always indent with 2 spaces.
  • Never open a paren/bracket/curly and then move to the next line.
  • Trailing parentheses/brackets/curlies should always go at the end of the last line, not on their own line.

So this code:

(def rules {
    :survival #{2 3}
    :birth #{3}
})

Becomes:

(def rules
  {:survival #{2 3}
   :birth #{3}})

Like Jérôme mentioned, def and defn should only ever occur at the "top level" of your program. It is for defining global functions.

For local definitions inside of a top-level definition, use let. For example, this code of yours:

(defn neighbors [cell]
    (def neighbors-of-origin #{(P -1 -1) (P -1 0) (P -1 1) (P 0 -1) (P 0 1) (P 1 -1) (P 1 0) (P 1 1)})
    (defn plus [cell1 cell2] {:x (+ (:x cell1) (:x cell2)) :y (+ (:y cell1) (:y cell2))})

    (map #(plus cell %) neighbors-of-origin))

Can be written more idiomatically like this:

(defn neighbors
  [cell]
  (let [neighbors-of-origin #{(P -1 -1)
                              (P -1 0)
                              (P -1 1)
                              (P 0 -1)
                              (P 0 1)
                              (P 1 -1)
                              (P 1 0)
                              (P 1 1)}
        plus (fn [cell1 cell2]
               {:x (+ (:x cell1) (:x cell2))
                :y (+ (:y cell1) (:y cell2))})]
    (map #(plus cell %) neighbors-of-origin)))

Or, (and this is nicer, in my opinion), you could promote the local definitions to top-level definitions, like this:

(def neighbors-of-origin
  #{(P -1 -1)
    (P -1 0)
    (P -1 1)
    (P 0 -1)
    (P 0 1)
    (P 1 -1)
    (P 1 0)
    (P 1 1)})

(defn plus
  [cell1 cell2]
  {:x (+ (:x cell1) (:x cell2))
   :y (+ (:y cell1) (:y cell2))})

(defn neighbors
  [cell]
  (map #(plus cell %) neighbors-of-origin))

You could define a Point record with :x and :y members, but in my opinion, that would be overkill for this simple purpose. A map would be simpler, like:

(def neighbors-of-origin
  #{{:x -1 :y -1}
    {:x -1 :y 0}
    {:x -1 :y 1}
    {:x 0 :y -1}
    {:x 0 :y 1}
    {:x 1 :y -1}
    {:x 1 :y 0}
    {:x 1 :y 1}})

(defn plus
  [cell1 cell2]
  {:x (+ (:x cell1) (:x cell2))
   :y (+ (:y cell1) (:y cell2))})

Records come in handy when you want to define a data type with custom behavior, like if you had some kind of protocol or interface that you wanted to implement for your Point type. But in this case, it looks like you just want to represent an X and Y value together as data, so a map works just fine for that purpose. (But I would use a vector instead; see below.)

When working with vectors/sequences, maps, and records, you can use destructuring to make your code more concise and easier to read:

(defn plus
  [{x1 :x y1 :y} {x2 :x y2 :y}]
  {:x (+ x1 x2)
   :y (+ y1 y2)})

All of this said: Whenever I deal with points in Clojure code, I tend to represent them as vectors of the X and the Y values, like [1 2]. So in this case, I would do:

(def neighbors-of-origin
  #{[-1 -1]
    [-1 0]
    [-1 1]
    [0 -1]
    [0 1]
    [1 -1]
    [1 0]
    [1 1]})

(defn plus
  [[x1 x2] [y1 y2]]
  [(+ x1 x2) (+ y1 y2)])

Your tests should live in a separate file with a namespace named the same thing as the implementation namespace, plus -test. In this case, your implementation namespace is com.nelkinda.game-of-life, so the test namespace should be com.nelkinda.game-of-life-test.

The usual Clojure project structure is to have src and test folders at the top level, so your project should look something like this:

.
├── deps.edn
├── src
│   └── com
│       └── nelkinda
│           └── game_of_life.clj
└── test
    └── com
        └── nelkinda
            └── game_of_life_test.clj

All .clj source files should begin with an ns form, so your test file should begin with something like this:

(ns com.nelkinda.game-of-life-test
  ...)

Including (use ...) in a Clojure project is considered unidiomatic. use is more of a utility function to use in your REPL during development. For what you're doing here, you can use :refer :all:

(ns com.nelkinda.game-of-life-test
  (:require [clojure.test              :refer :all]
            [com.nelkinda.game-of-life :refer :all]))

However, a lot of Clojure programmers recommend against :refer :all (and many recommend against :refer in general) because it hides away the source of each of the functions being used from other namespaces, and it can be hard to understand where each function comes from as you're looking at the code, especially when you are requiring many namespaces.

A good best practice is to explicitly qualify every function that you're using from other namespaces by using :as to give the namespace an alias: (:require [some-namespace :as s]) after which you can refer to the vars from that namespace like s/some-var.

There are some rare exceptions where it is conventional to refer specific functions in from another namespace, and clojure.test is one of those.

I've never seen Given and Then in Clojure tests before... are those provided by clojure.test? At any rate, you'll usually see tests written using deftest, testing and is.

Another thing that sticks out to me is that your implementation is all pure functions, but your tests are stateful, with a universe atom that you reset between every test. You could really simplify this by testing the expected results of calling the pure functions.

So, tying this all together, I would write your tests like this:

(ns com.nelkinda.game-of-life-test
  (:require [clojure.test              :refer [deftest testing is]]
            [com.nelkinda.game-of-life :as    life]))

(deftest game-of-life-tests
  (testing "generation logic"
    (let [universe a-specific-example]
      (is (= expected-next-generation (life/next-generation universe))))
    (let [universe another-example]
      (is (= expected-next-generation (life/next-generation universe))))))

I like your idea of generating examples from readable specs. You could bring that in by doing something like this:

(defn parse
  [spec]
  ...)

(def specs
  ["spec 1"
   "spec 2"
   "spec 3"])

(deftest game-of-life-tests
  (testing "generation logic"
    (doseq [spec specs]
      (let [universe (parse spec)]
        (is (= expected-next-generation (life/next-generation universe)))))))

That's all I've got. I hope this helps!

\$\endgroup\$
2
  • \$\begingroup\$ Thanks for the elaborate feedback. About the directory structure, I didn't get why you were pointing this out, as far as I can tell, the directory structure is as per your description. Can you elaborate on the point? \$\endgroup\$ Feb 15 at 15:05
  • 1
    \$\begingroup\$ Ah, I didn't see the link to the repo! I was just looking at the code you posted here. I mentioned the directory structure just in case it was helpful to new Clojurists. \$\endgroup\$ Feb 16 at 16:37

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.