如何提高大型lisp函数的可读性

Question

我的主要方法(remove-random-edge)看起来很难读懂。我是新来的，所以希望能有任何关于如何改进代码的建议。

(defun find-node (node graph)
  (find-if #'(lambda (i) (eql node (first i))) graph))

;; Input, graph, is in form ((from-1 to-1 to-2 to-3 ...)
;;                           (from-2 to-4 to-5 to-6 ...) ...)
;; where from-n and to-n are integers.
(defun remove-random-edge (graph)
  "Remove one random edge from a graph given as adjacency list."
  (let* ((node-list-1 (elt graph (random (length graph))))
         (node-1 (first node-list-1))
         (destinations (remove-duplicates (rest node-list-1)))
         (node-2 (elt destinations (random (length destinations))))
         (node-list-2 (find-node node-2 graph)))
    (flet ((replace-tail-for-head (node) (if (eql node node-2) node-1 node))
           (is-head-p (node) (eql node-1 node))
           (is-tail-p (node) (eql node-2 node))
           (starts-with-tail-p (nodes) (eql node-2 (first nodes))))
      (setf (rest node-list-1) (concatenate 'list 
                                 (rest node-list-1)
                                 (remove-if #'is-head-p (rest node-list-2)))) 
      (loop for node in (remove-duplicates (rest node-list-2))
            with match
            with repcd 
            do (setf match (find-node node graph))
            do (setf repcd (if (eql node node-1)
                             (remove-if #'is-tail-p (rest match))
                             (map 'list #'replace-tail-for-head (rest match))))
            do (setf (rest match) (sort repcd #'<))) 
      (remove-if #'starts-with-tail-p graph))))

UPD:应用了审查意见：

(defun remove-random-edge (graph)
  "Remove one random edge from a graph given as adjacency list."
  (let* ((head-list (elt graph (random (length graph))))
         (head (first head-list))
         (destinations (remove-duplicates (rest head-list)))
         (tail (elt destinations (random (length destinations))))
         (tail-list (assoc tail graph)))
    (flet ((replace-tail-for-head (node) (if (eql node tail) head node)))
      (setf (rest head-list) (concatenate 'list 
                               (rest head-list)
                               (remove head (rest tail-list)))) 
      (loop for node in (remove-duplicates (rest tail-list))
            for match = (assoc node graph)
            do (setf (rest match) (if (eql node head)
                                    (remove tail (rest match))
                                    (mapcar #'replace-tail-for-head (rest match)))))
      (remove tail graph :key #'first))))

在此之前：

15.109 seconds of real time
50,245,719,578 processor cycles
767,039,256 bytes consed

之后：

2.312 seconds of real time
7,665,669,728 processor cycles
778,172,816 bytes consed

Answer 1

浅表

您的find-node实际上是(几乎) assoc，或者，如果您愿意的话，是(find node graph :key #'first)。

使用mapcar而不是map 'list，因为它使意图更加清晰(区别在于mapcar只接收列表和map任何序列)。

你只需要一个do在loop；你甚至可以折叠你所有的setfs成一个(setf a b c d e f)。

在loop中，with应该(在文体上)位于for之前。

深度

您的代码看起来太复杂了，无法完成它的工作。

您的flet本地函数中没有一个是真正必要的。例如，(remove-if #'is-head-p ...)应该是(remove node-1 ...)，(remove-if #'starts-with-tail-p ...)应该是(remove node-2 ... :key #'first)；这将使代码更快、更简洁。

你的loop应该是

(loop for node in (remove-duplicates (rest node-list-2))
      for match = (find-node node graph)
      for repcd = (if (eql node node-1)
                      (remove-if #'is-tail-p (rest match))
                      (map 'list #'replace-tail-for-head (rest match))))
      do (setf (rest match) (sort repcd #'<))) 

General

通常情况下，最好使用您需要的最具体的功能。例如，对简单变量使用setq (而不是支持一般引用的setf )。在处理列表时使用mapcar而不是map 'list (与向量相反)。