Clojure code for generating HAL JSON responses

I wanted to use hal in one of my personal Clojure projects, but unfortunately the only library I could find (halresource) was somewhat out of date when compared to the current specification. So, I decided to roll my own, which works fine for me personally, but I'd like to make it publicly available. However, I don't have a good grasp on whether the code is good and the functions are straightforward and clear to others - hence this posting.

The code uses maps as representations of hal resources, and since json maps very well to Clojure, the maps can be directly serialized to JSON with Cheshire. The intended usage would be something like:

(-> (new-resource "somewhere.com")
    (add-property :king "The King Of Somewhere")
    (add-properties {:country "The Kingdom Of Somewhere" 
                     :city "Some City"})
    (add-link "embassy" "/embassy")
    (add-embedded-resources :adjacent_countries
                            (new-resource "nowhere.com")
                            (-> (new-resource "azurecity.com")
                                (add-property :ruler "Lord Shojo"))))

which would produce:

{
  "_embedded": {
    "adjacent_countries": [
      {
        "_links": {
          "self": {
            "href": "nowhere.com"
          }
        }
      },
      {
        "ruler": "Lord Shojo",
        "_links": {
          "self": {
            "href": "azurecity.com"
          }
        }
      }
    ]
  },
  "city": "Some City",
  "country": "The Kingdom Of Somewhere",
  "king": "The King Of Somewhere",
  "_links": {
    "embassy": {
      "href": "\/embassy"
    },
    "self": {
      "href": "somewhere.com"
    }
  }
}

Aside from general advice on best practices for writing Clojure code, I'd like to know:

• Is my usage of {:pre} sane and not abusive? Is there a better way to ensure that the input into my functions conforms to the hal specification without using defrecord or some other custom non-map data structure? Or, should I just

First of all, great idea -- you should consider contributing to the halresource library to bring it up to date!

Here are my critiques:

Structure of data

The structure you have chosen to represent links, while truer to their HAL representations, leads to unnecessarily complicated code. What you have is something like this:

{"embassy" {:href "/embassy" :templated true}}

The halresource version of this, which I think is better because it's simpler, is this:

{:rel "embassy" :href "/embassy" :templated true}

The difference might look negligible, but the halresource version leads to simpler code because you're not having to deal with nested map structures. You can easily grab any value from a one-level map by its keyword. Say you want to grab the href field of the link above -- for the second example it's (:href link), but for the first example you have to do something like (get-in link ["embassy" :href]) or (:href (get link "embassy")). It's also confusing because you should generally avoid using strings as keys in a map -- prefer keywords instead.

I think I understand why you have opted for the first method -- it looks more like HAL's representation of links. But the key thing here is that you can transform your data structures however you'd like, once it comes time to translate your Clojure data structure to HAL format; until then, you're doing a lot of work on the data while it's still in the form of Clojure data structures, so I think it makes more sense to have it in a simpler, more intuitive form.

Accessing map fields

While I'm on the topic of maps, I need to point out that you are using them incorrectly. You're trying to treat maps as sequential structures, which they are not. A map is an unsorted (i.e. non-sequential) data structure; it doesn't make sense to grab the "first" or "second" item of a map, as a map does not have any particular order. Clojure won't throw an error when you treat maps this way, but the results that you get will be unreliable. Instead, you need to grab items from a map by key. For example, to get the href value of link, use (:href link) or (link :href) (in Clojure, you can conveniently use a map as a function to look up its argument, or a keyword as a function to look itself up in its argument).

Pre-conditions

I think that using pre-/post-conditions is a good idea here, at least for your add-* functions. I like pre-/post-conditions because they tend to make code more concise and they make it extra clear in reading the code what kind of arguments are supposed to be passed into each function, so they also serve the purpose of self-documenting code.

That being said, I think you could benefit from modifying your approach to validating links slightly. This goes hand-in-hand with having a simpler representation of links as ordinary maps, as I just noted above. The way you have it now, you're validating links via specific pre-conditions attached to the new-link function. The problem with this approach is that it assumes new-link is used to create each and every link! If the user decides to create his own link manually via a map literal, then there is nothing in place to validate that link.

I think a better solution would be to get rid of your new-link function altogether and let links be represented as ordinary, one-level maps. Validation should be done as a part of the add-link function, which ought to validate links before it adds them -- you can of course do this with a pre-condition. I would also simplify your pre-condition down to a single helper function, valid-link?, and do something like this:

(defn minimally-templated?
  "Checks whether a given href is minimally templated; full documentation is here:
   http://tools.ietf.org/html/rfc6570"
  [href]
  (re-find #"\{.+\}" href))

(defn valid-link?
  "Checks that a given link is valid. Links are represented as maps, e.g.,

      {:rel 'embassy', :href '/embassy', :templated true}

   See the 'link-properties' set above for other possible properties.

   If the :templated property is true, the href must be minimally templated."
  [link]
  (and
    (every? link-properties (keys link)) 
    (or 
      (not (:templated link))
      (minimally-templated? (:href link)))))

; (Note: you don't actually have to check whether the link has a `:curies` property,      
; because `:curies` is not included in your set of acceptable `link-properties`.)

(defn add-link
  "Docstring foo bar baz & quux." 
  ([resource link]
    {:pre [(valid-link? link)]}
    (update-in resource [:_links] #((fnil conj []) % link)))
  ([resource rel href & properties]
    {:pre [(even? (count properties))]}
    (reduce add-link resource (partition 2 properties))))

(A note about the above code: in trying to figure out the best way to refactor your add-link function, I ended up settling on (update-in resource [:_links] #((fnil conj []) % link))), which is essentially identical to the code used in h

StackExchange Code Review Q#53849, answer score: 2