2
votes

I am working through the excellent book Let Over Lambda, and I am trying to port the Common Lisp code for defunits over to Clojure.

The following generates a macro that is supposed to take

(defn defunits-chaining [u units prev]
  (if (some #(= u %) prev)
    (throw (Throwable. (str u " depends on " prev))))
  (let [spec (first (filter #(= u (first %)) units))]
    (if (nil? spec)
      (throw (Throwable. (str "unknown unit " u)))
      (let [chain (second spec)]
        (if (list? chain)
          (* (first chain)
             (defunits-chaining
               (second chain)
               units
               (cons u prev)))
          chain)))))

(defmacro defunits [quantity base-unit & units]
  `(defmacro ~(symbol (str "unit-of-" quantity))
     [valu# un#]
     `(* ~valu#
         ~(case un#
           ~base-unit 1
            ~@(map (fn [x]
                     `(~(first x)               ;; <-- PRETTY SURE IT'S THIS `(
                       ~(defunits-chaining
                          (first x)
                          (cons `(~base-unit 1)
                                (partition 2 units))
                          nil)))
                   (partition 2 units))))))

(defunits time s m 60 h 3600)

and turn it into a macro that can be called like

(unit-of-time 4 h)

and give the results in the base unit (seconds in this case). I think is the problem is a Clojure/CL api change in "case". "Case" in CL looks like this:

(case 'a (('b) 'no) (('c) 'nope) (('a) 'yes))

but in clojure...

(case 'a 'b 'no 'c 'nope 'a 'yes)

HOW CONVENIENT. I changed my anon function in the nested defmacro, but it keeps generating like

(case un#
  s 1
  (m 60)
  (h 3600)

How can I prevent those outer parens?

3

3 Answers

6
votes

If you wrap your map in a flatten, it should produce the results your looking for.

(defmacro defunits [quantity base-unit & units]
  `(defmacro ~(symbol (str "unit-of-" quantity))
     [valu# un#]
     `(* ~valu#
         ~(case un#
           ~base-unit 1
            ~@(flatten ;; <- changed this
                (map (fn [x]
                       `(~(first x)
                         ~(defunits-chaining
                            (first x)
                            (cons `(~base-unit 1)
                                  (partition 2 units))
                            nil)))
                   (partition 2 units)))))))

What's going on: In your initial implementation, your map is returning something a list of lists when what you need is a list of atoms. Flatten takes an arbitrarily deep list of lists and turns it into a single list of values.

Another approach would be to use a reduce rather than a map:

(defmacro defunits [quantity base-unit & units]
  `(defmacro ~(symbol (str "my-unit-of-" quantity))
     [valu# un#]
     `(* ~valu#
         ~(case un#
           ~base-unit 1
            ~@(reduce (fn [x y] ;; <- reduce instead of map
                        (concat x ;; <- use concat to string the values together
                              `(~(first y)
                                ~(defunits-chaining
                                   (first y)
                                   (cons `(~base-unit 1)
                                         (partition 2 units))
                                   nil))))
                      '()
                      (partition 2 units))))))

This would avoid creating the list of lists in the first place as reduce rolls up all the passed in values into a single result.

Better yet, (Thanks to amalloy for spotting this one), there's mapcat:

(defmacro defunits [quantity base-unit & units]
  `(defmacro ~(symbol (str "unit-of-" quantity))
     [valu# un#]
     `(* ~valu#
         ~(case un#
            ~base-unit 1
            ~@(mapcat (fn [x] ;; <----- mapcat instead of map is the only change
                        `(~(first x)
                          ~(defunits-chaining
                             (first x)
                             (cons `(~base-unit 1)
                                   (partition 2 units))
                             nil)))
                      (partition 2 units))))))

Mapcat effectively does the same thing as the reduce version but implicitly handles the concat for you.

4
votes

Just change your map to mapcat.

(defmacro defunits [quantity base-unit & units]
  `(defmacro ~(symbol (str "unit-of-" quantity))
     [valu# un#]
     `(* ~valu#
         ~(case un#
            ~base-unit 1
            ~@(mapcat (fn [x] ;; <----- mapcat instead of map is the only change
                        `(~(first x)
                          ~(defunits-chaining
                             (first x)
                             (cons `(~base-unit 1)
                                   (partition 2 units))
                             nil)))
                      (partition 2 units))))))

To elaborate, and respond to the currently-accepted answer: flatten is never, never, never right (with rare exceptions for use by experts only). At best it's a band-aid that will get you the right results for simple inputs but fail on complex inputs. Instead, it is usually right to use apply concat to flatten a list by one level, or use mapcat instead of map to generate an already-flatter list, or perhaps something more involved and specialized for a particularly convoluted data structure. Here, a simple mapcat is all that is needed.

I suppose I should note that the defunits macro's inputs are so tightly constrained (they really must be symbols and numbers), in practice there will be no input that flatten will produce the wrong output for. But flatten is a bad habit to get into, and results in longer, more complicated code anyway.

3
votes

You can concatenate the parenthesized pairs (using, say, apply concat) or use amalloy's answer (the best idea).

You could also construct your return form using seq functions:

(defmacro defunits [...]
  (let [macro-name (symbol ...)
        valsym (gensym "val__")
        unitsym (gensym "unit__")]
    (list `defmacro             ; using ` so the symbol gets ns-qualified
          macro-name
          [valsym unitsym]
          (make-body-expression ...)))) ; define appropriately

I think this makes sense here, since you end up unquoting pretty much everything anyway, with two levels of syntax-quote; it is largely a matter of taste though.