Tag: do’s and don’ts

This is a guide on naming Clojure functions. There are exceptions to every rule. When you’re defining something based on natural language, there are more exceptions than rules. I break these rules more often than I follow them. This guide is just a starting point for thinking about how to name things.

(ns products)

;; Bad, redundant:
(defn product-price [product]
  ;; ...
  )

;; Good:
(defn price [product]
  ;; ...
  )

(defn shipping-label
  "Returns a new label to ship product to customer."
  [customer product]
  (let [address (address customer)
        weight (weight product)
        supplier (supplier product)]
    {:from (address supplier)  ; oops, 'address' clashes!
     :to address
     :weight weight}))

(defn shipping-label
  "Returns a new label to ship product to customer."
  [customer product]
  (let [address (mailing/address customer)
        weight (product/weight product)
        supplier (product/supplier product)]
    {:from (mailing/address supplier)  ; OK
     :to address
     :weight weight}))

(defn foo
  "Returns a function to compute foo of value."
  [option]
  (fn [value]
    ;;... do stuff with value ...
    ))

(defn computation
  "Does stuff with values."
  [option values]
  (->> values
       (map (foo option))  ; look at me!
       ;; ...
       ))

(defn foo
  "Returns the foo of value"
  [value option]
  ;; ... 
  )

(defn computation [option values]
  (->> values
       (map #(foo % option))  ; I see what this does
       ;; ...
       ))

(defn foo-fn
  "Returns a function to compute the foo of a value."
  [param]
  (fn [value]
    ;; ...
    ))

This is probably my number one Clojure Don’t.

Laziness is often useful. It allows you to express “infinite” computations, and only pay for as much of the computation as you need.

Laziness also allows you to express computations without specifying when they should happen. And that’s a problem when you add side-effects.

By definition, a side-effect is something that changes the world outside your program. You almost certainly want it to happen at a specific time. Laziness takes away your control of when things happen.

So the rule is simple: Never mix side effects with lazy operations.

For example, if you need to do something to every element in a collection, you might reach for map. If thing you’re doing is a pure function, that’s fine. But if the thing you’re doing has side effects, map can lead to very unexpected results.

For example, this is a common new-to-Clojure mistake:

(take 5 (map prn (range 10)))

which prints

0
1
2
3
4
5
6
7
8
9

This is the old “chunked sequence” conundrum. Like many other lazy sequence functions, map has an optimization which allows it to evaluate batches of 32 elements at a time.

Then there’s the issue of lazy sequences not being evaluated at all. For example:

(do (map prn [0 1 2 3 4 5 6 7 8 9 10])
    (println "Hello, world!"))

which prints only:

Hello, world!

You might get the advice that you can “force” a lazy sequence to be evaluated with doall or dorun. There are also snippets floating around that purport to “unchunk” a sequence.

In my opinion, the presence of doall, dorun, or even “unchunk” is almost always a sign that something never should have been a lazy sequence in the first place.

Only use pure functions with the lazy sequence operations like map, filter, take-while, etc. When you need side effects, use one of these alternatives:

• doseq: good default choice, clearly indicates side effects
• run!: new in Clojure 1.7, can take the place of (dorun (map ...))
• reduce, transduce, or something built on them

The last requires some more explanation. reduce and transduce are both non-lazy ways of consuming sequences or collections. As such, they are technically safe to use with side-effecting operations.

For example, this composition of take and map:

(transduce (comp (take 5)
                 (map prn))
           conj
           []
           (range 10))

only prints 5 elements of the sequence, as requested:

0
1
2
3
4

The single-argument version of map returns a transducer which calls its function once for each element. The map transducer can’t control when the function gets evaluated — that’s in the hands of transduce, which is eager (non-lazy). The single-argument take limits the reduction to the first five elements.

As a general rule, I would not recommend using side-effecting operations in transducers. But if you know that the transducer will be used only in non-lazy operations — such as transduce, run!, or into — then it may be convenient.

(defn operation [input]
  ;; do something with input, return result
  (str "Result for " input))

(prn (into #{}
           (comp (take 3)
                 (map operation))
           (range 100)))

reduce, transduce, and into are useful when you need to collect the return value of the side-effecting operation.

Today’s Clojure Don’t is the opposite side of the coin to the heisenparameter.

If you have an operation on a single object, you don’t need to define another version just to operate on a collection of those objects.

That is, if you have a function like this:

(defn process-thing [thing]
  ;; process one thing
  )

There is no reason to also write this:

(defn process-many-things [things]
  (map process-thing things))

The idiom “map a function over a collection” is so universal that any Clojure programmer should be able to write it without thinking twice.

Having a separate definition for processing a group of things implies that there is something special about processing a group instead of a single item. (For example, a more efficient batch implementation.) If that’s the case, then by all means write the batch version as well. But if not, then a function like process-many-things just clutters up your code while providing no benefit.

A short Clojure don’t for today. This one is my style preference.

You have a single expression which should run if a condition is true, otherwise return nil.

Most Clojure programmers would probably write this:

(when (condition? ...)
  (then-expression ...))

But you could also write this:

(if (condition? ...)
  (then-expression ...)
  nil)

Or even this, because the “else” branch of if defaults to nil:

(if (condition? ...)
  (then-expression ...))

There’s an argument to be made for any one of these.

The second variant, if ... nil, makes it very explicit that you want to return nil. The nil might be semantically meaningful in this context instead of just a “default” value.

Some people like the third variant, if with no “else” branch, because they think when is only for side-effects, leaving the single-branch if for “pure” code.

But for me it comes down, as usual, to readability.

The vast majority of the time, if contains both “then” and “else” expressions.

Sometimes a long “then” branch leaves the “else” branch dangling below it. I’m expecting this, so when I read an if my eyes automatically scan down to find the “else” branch.

If I see an if but don’t find an “else” branch, I get momentarily confused. Maybe a line is missing or the code is mis-indented.

Likewise, if I see an if explicitly returing nil, it looks like a mistake because I know it could be written as when. This is a universal pattern in Clojure: lots of expressions (cond, get, some) return nil as their default case, so it’s jarring to see a literal nil as a return value.

So my preferred style is the first version. In general terms:

An if should always have both “then” and “else” branches.
Use when for a condition which should return nil in the negative case.

A pattern I particularly dislike: Function parameters which may or may not be collections.

Say you have a function that does some operation on a batch of inputs:

(defn process-batch [items]
  ;; ... do some work with items ...
  )

Say further that, for this process, the fundamental unit of work is always a batch. Processing one thing is just a batch size of one.

Lots of processes are like this: I/O (arrays of bytes), database APIs (transactions of rows), and so on.

But maybe you have lots of code that mostly deals with one thing at a time, and only occasionally makes a larger batch. In the name of “convenience,” people write things like this:

(defn wrap-coll
  "Wraps argument in a vector if it is not already a collection."
  [arg]
  (if (coll? arg)
    arg
    [arg]))

(defn process
  "Processes a single input or a collection of inputs."
  [input]
  (process-batch (wrap-coll input)))

This is prevalent in dynamically-typed languages of all stripes. I think it’s a case of mistakenly choosing convenience over clarity.

This leads easily to mistakes like iterating over a collection, calling process on each element, when the same work could be done more efficiently in a batch.

Now imagine reading some code when you encounter a call to this function:

(process stuff)

Is stuff a collection or a single object? Who knows?

When you read code, there’s a kind of ad-hoc, mental type-inference going on. This is true regardless of what typing scheme your language uses. Narrowing the range of possible types something can be makes it easier to reason about what type it actually is.

The more general principle:
Be explicit about your types even when they’re dynamic.

If the operation requires a collection, then pass it a collection every time.

A “helper” like wrap-coll saves you a whopping two characters over just wrapping the argument in a literal vector, at the cost of lost clarity and specificity.

If you often forget to wrap the argument correctly, consider adding a type check:

(defn process-batch [items]
  {:pre [(coll? items)]}
  ;; ... 
  )

If there actually are two distinct operations, one for a single object and one for a batch, then they should be separate functions:

(defn process-one [item]
  ;; ...
  )

(defn process-batch [items]
  ;; ...
  )

Another Clojure don’t today. This one is a personal style preference, but I’ll try to back it up.

Say you want to define a function with a mix of required and optional arguments. I’ve often seen this:

(defn foo [a & [b]]
  (println "Required argument a is" a)
  (println "Optional argument b is" b))

This is a clever trick. It works because & [b] destructures the sequence of arguments passed to the function after a. Sequential destructuring doesn’t require that the number of symbols match the number of elements in the sequence being bound. If there are more symbols than values, they are bound to nil.

(foo 3 4)
;; Required argument a is 3
;; Optional argument b is 4
;;=> nil

(foo 9)
;; Required argument a is 9
;; Optional argument b is nil
;;=> nil

I don’t like this pattern for two reasons.

One. Because it’s variable arity, the function foo accepts any number of arguments. You won’t get an error if you call it with extra arguments, they will just be silently ignored.

(foo 5 6 7 8)
;; Required argument a is 5
;; Optional argument b is 6
;;=> nil

Two. It muddles the intent. The presence of & in the parameter vector suggests that this function is meant to be variable-arity. Reading this code, I might start to wonder why. Or I might miss the & and think this function is meant to be called with a sequence as its second argument.

A couple more lines make it clearer:

(defn foo
  ([a]
   (foo a nil))
  ([a b]
   (println "Required argument a is" a)
   (println "Optional argument b is" b)))

The intent here is unambiguous: The function takes either one or two arguments, with b defaulting to nil. Trying to call it with more than two arguments will throw an exception, telling you that you did something wrong.

And one more thing: it’s faster. Variable-arity function calls have to allocate a sequence to hold the arguments, then go through apply. Timothy Baldridge did a quick performance comparison showing that calls to a function with multiple, fixed arities can be much faster than variable-arity (varargs) function calls.

Some more Clojure Do’s and Don’ts for you. This week: record constructors.

(defrecord Foo [a b])

(Foo. 1 2)
;;=> #user.Foo{:a 1, :b 2}

(defrecord Foo [a b])

(->Foo 1 2)
;;=> #user.Foo{:a 1 :b 2}

(map->Foo {:b 4 :a 3})
;;=> #user.Foo{:a 3, :b 4}

(deftype Bar [c d])

(->Bar 5 6)
;;=> #<Bar user.Bar@2168aeae>

(defrecord Customer [id name phone email])

(defn customer
  "Creates a new customer record."
  [{:keys [name phone email]}]
  {:pre [(string? name)
         (valid-phone? phone)
         (valid-email? email)]}
  (->Customer (next-id) name phone email))