Is it necessary for me to use `cl-labels` in this grouping function, when plain recursion does the job as well?

Question

So I'm reading through some tutorials with Common Lisp, and I came across this function, group:

(defun group (source n)
  (if (zerop n) (error "zero length"))
  (labels ((rec (source acc)
             (let ((rest (nthcdr n source)))
               (if (consp rest)
                   (rec rest (cons
                               (subseq source 0 n)
                               acc))
                   (nreverse
                     (cons source acc))))))
    (if source (rec source nil) nil)))

After some futzing around, trying to understand what this does (it basically takes a flat list, recurses through it, and then groups them into groups of GROUP-SIZE), I've come up with two alternatives.

This is the version that uses cl-labels, and is basically a straight-up translation of the above group function (along with some additional modifications):

(defun *hx/group-label (sources group-size)
  (if (> 0 group-size)
      (error "Invalid GROUP-SIZE %d: GROUP-SIZE must be greater than zero." group-size)
    (cl-labels ((rec (source accumulator)
                     (let ((others (nthcdr group-size source)))
                       (if (consp others)
                           (rec others (cons
                                        (subseq source 0 group-size)
                                        accumulator))
                         (nreverse
                          (cons source accumulator))))))
      (if sources
          (rec sources nil)
        nil))))

But then I realized... hang on, why do I need to use cl-labels? And so I came up with this version:

(defun *hx/group-recurse (sources group-size &optional accumulator)
  (if (> 0 group-size)
      (error "Invalid GROUP-SIZE %d: GROUP-SIZE must be greater than zero." group-size)
    (let ((others (nthcdr group-size sources)))
      (if (consp others)
          (*hx/group-recurse others group-size (cons
                                                (subseq sources 0 group-size)
                                                accumulator))
        (nreverse
         (cons sources accumulator))))))

What I'd like to know, however, is why it would be necessary to use cl-labels? I'm already using -*- lexical-binding: t -*-, so I don't see the advantages in terms of lexical scope (which cl-labels does, according to the documentation), and they both appear to be functionally similar.

But then again, going through the questions that this query comes up, I'm seeing advice that tells me to not recurse when loops will do. What would be the best way to do this, then?

FWIW, I'm planning to use this function to turn something like this in my init file:

(add-hook 'text-mode-hook (lambda ()
                            (visual-line-mode t)))
(add-hook 'prog-mode-hook (lambda ()
                            (visual-line-mode t)))
(add-hook 'visual-line-mode-hook #'visual-fill-column-mode)
(add-hook 'Info-mode-hook (lambda ()
                                   (setq visual-fill-column-width 80)
                                   (visual-fill-column-mode)))

Into something a little more concise, like:

(add-multiple-hooks (<group-function> (prog-mode-hook <function-def>
                                       text-mode-hook <function-def>
                                       ...)))

Anyone's insights on this would be highly appreciated!

By the way, this function comes with Emacs 25+ (also [available from GNU ELPA](http://elpa.gnu.org/packages/seq.html) for older versions) as `seq-partition`. — npostavs, Sep 08 '18 at 15:02
Oh! That _is_ good to know. A little annoying, because I spent quite a bit of time reinventing a wheel. — Tariq Kamal, Sep 10 '18 at 16:28
Well, now you know something more about wheelbuilding, so you'll be able to tell a good wheel from a bad one. :) — npostavs, Sep 11 '18 at 03:47

score 1 · Accepted Answer · answered Sep 08 '18 at 10:16

It's not necessary to use cl-labels, but note that recursing over *hx/group-recurse imposes some overhead in that it must test (> 0 group-size) every call, when obviously that is not needed beyond the original call. Using labels keeps the recursive part simpler, which is generally desirable. For such reasons it's very common to see a pair of functions used for recursive definitions.

I'm seeing advice that tells me to not recurse when loops will do. What would be the best way to do this, then?

There are lots of options, but you might do something like this:

(defun *hx/group (sources group-size)
  (if (> 0 group-size)
      (error "Invalid GROUP-SIZE %d: GROUP-SIZE must be greater than zero." group-size)
    (let (result group)
      (while sources
        (setq group nil)
        (dotimes (_i group-size)
          (when sources
            (push (pop sources) group)))
        (push (nreverse group) result))
      (nreverse result))))

Ah, you're right! I didn't notice that. So what you're saying is, perform the initial test to winnow out the possible causes of error, and then continue with the recursion, right? That makes a lot of sense. Thank you! — Tariq Kamal, Sep 08 '18 at 14:00
Also worth mentioning that the added `&optional accumulator` clutters the function's interface with an implementation detail. — npostavs, Sep 08 '18 at 15:04

Is it necessary for me to use `cl-labels` in this grouping function, when plain recursion does the job as well?

1 Answers1