Dolist not modifying the list

Question

I'm trying to prepend each substring of split-string with numbering (1.,2.,3.,..) then return all the substrings concatenated with a newline

(defun japanese-get-definition (dictentry)
  "Get a definition from a dictionary entry."
  (let* ((index 1)
         (definitions))
    (save-match-data
      (and (string-match "/\\(.*?\\)$" dictentry)
           (setq definitions (split-string (match-string 1 dictentry) "/" t))
           (dolist (definition definitions)
             (setq definition (concat (int-to-string index) ". " definition))
             (setq index (+ index 1)))
           (mapconcat 'identity
                      definitions
                      "\n"))
      )
    )
  )

(japanese-get-definition "職業 [しょくぎょう] /(n,adj-no) occupation/business/")

So the result would looks like

1. (n,adj-no) occupation
2. business

Currently though the function is returning nil. Is dolist the right function to use? This would probably be very easy if I just use a new list, but I'm hoping to use learn the best way to do this (which I think (could be wrong) is modifying the existing list so less memory is used)

Answer 1

1. The dolist can have no effect on definitions. If you remove it from your function you will see that you get a result (but not exactly what you wanted). The mapconcat is evaluated and does not return nil.

2. The result of (setq definitions (split-string (match-string 1 dictentry) "/" t)) in your test example is ("(n,adj-no) occupation" "business").

3. The mapconcat applied to that would give "(n,adj-no) occupation\nbusiness"

4. dolist returns nil, so the mapconcat is never evaluated.

5. If you wrap your dolist with (progn ... t) then the mapconcat will be evaluated.

But your main problem is thinking that dolist modifies the original list. As explained in the post you cited in your other question, you can use dolist to modify individual elements of the list but not the list itself.

For example, if the list had particular strings or lists as its elements, you could modify those individual objects. (The individual elements of the list must of course be modifiable, for you to modify them. You cannot modify numbers, for example.)

But in your case you want to modify the list itself. You want to replace a given element with a completely different value -- that is, replace it in the list. That's not the same as modifying an element (which changes the value globally). You cannot do that with dolist.

Here's one quick, ugly-hacky way to do what you want. It doesn't change your code much. It just replaces the dolist by some code that modifies definitions.

(defun japanese-get-definition (dictentry)
  "Get a definition from a dictionary entry."
  (let* ((index 1)
         (definitions))
    (save-match-data
      (and (string-match "/\\(.*?\\)$" dictentry)
           (setq definitions (split-string (match-string 1 dictentry) "/" t))
           (let ((defs  definitions))
             (while defs
               (setcar defs (concat (int-to-string index) ". " (car defs)))
               (setq index  (1+ index)
                     defs   (cdr defs)))
             t)
           (mapconcat 'identity definitions "\n")))))

This is slightly simpler, but still not very pretty:

(defun japanese-get-definition (dictentry)
  "Get a definition from a dictionary entry."
  (let* ((index       1)
         (definitions ()))
    (save-match-data
      (when (string-match "/\\(.*?\\)$" dictentry)
        (setq definitions  (split-string (match-string 1 dictentry) "/" t))
        (let ((defs  definitions))
          (while defs
            (setcar defs (concat (int-to-string index) ". " (car defs)))
            (setq index  (1+ index)
                  defs   (cdr defs))))
        (mapconcat 'identity definitions "\n")))))

Answer 2

Thank you Drew for the explanation on what I was doing wrong. Another way to approach the problem (thanks tali713) is with a lambda in mapconcat

(defun japanese-get-definition (dictentry)
  "Get a definition from a dictionary entry."
  (let ((index 0))
    (mapconcat (lambda (str) (format "%s. %s" (incf index) str))
               (split-string dictentry "/" t)
               "\n")))

for a much cleaner solution.