How to modularize the common functionality of two

Question

I use one Lisp function to compile the current Java file and another to run it. Both functions use the same functionality to get the package name and the appropriate directory to compile/run from (this functionality was posted as an answer to another question). I would like to modularize the common functionality of these functions, i.e. from (let* to but not including (cd. How can I do this?

As bonus I would like to undo the changing of directory (the cd:ing) as the functions are done to avoid unexpected behaviour such as finding files (C-x C-f) starting in the parten directory. It has been suggested that this can be achieved by unwind-protect.

(add-hook 'java-mode-hook
      (lambda ()

    (defun java-compile-current-file ()
      "Compiles the current file with javac"
      (interactive)
      (let* ((package (save-excursion
                (goto-char (point-min))
                (when (re-search-forward "^\\s *package\\s +\\(.*\\);" (point-max) t)
                  (match-string 1))))
         (directory (file-name-directory (buffer-file-name)))
         sub-dirs)

        (if directory
        (setq directory (file-truename directory))
          (error "Current buffer is not visiting a file"))

        (when package
          (setq sub-dirs (reverse (split-string package "\\.")))

          (while sub-dirs
        (if (string-match (concat "^\\(.*/\\)" (regexp-quote (car sub-dirs)) "/$") directory)
            (setq directory (match-string 1 directory)
              sub-dirs (cdr sub-dirs))
          (error "Package does not match directory structure"))))

        (cd directory)
        (compile
         (concat "javac -Xlint:all " ; Tog bort -Werror från
                    ; argumenten. För
                    ; gnälligt!
             (if package (concat package "/") "")
             (file-name-nondirectory (buffer-file-name))))))

    (local-set-key [(f8)] 'java-compile-current-file)

    ;; https://stackoverflow.com/a/12548762/789593
    (defun java-run-current-file ()
      "Runs the java program the current file corresponds to"
      (interactive)
      (let* ((package (save-excursion
                (goto-char (point-min))
                (when (re-search-forward "^\\s *package\\s +\\(.*\\);" (point-max) t)
                  (match-string 1))))
         (directory (file-name-directory (buffer-file-name)))
         sub-dirs)

        (if directory
        (setq directory (file-truename directory))
          (error "Current buffer is not visiting a file"))

        (when package
          (setq sub-dirs (reverse (split-string package "\\.")))

          (while sub-dirs
        (if (string-match (concat "^\\(.*/\\)" (regexp-quote (car sub-dirs)) "/$") directory)
            (setq directory (match-string 1 directory)
              sub-dirs (cdr sub-dirs))
          (error "Package does not match directory structure"))))

        (cd directory)
        (shell-command
         (concat "java "
             (if package (concat package ".") "")
             (file-name-sans-extension
              (file-name-nondirectory (buffer-file-name)))))))

    (local-set-key [(f7)] 'java-run-current-file)))

Answer 1

Here is a crude, untested, refactoring of the code. It's not particularly elegant or idiomatic, but should at least be enough to get you started.

;; 1. Refactored `java-get-package' into its own `defun'
;; 2. Refactored `java-package-directory' into its own `defun'
;; 3. Broke out `defun's outside of `java-mode-hook'
;; 4. Use (let ((default-directory ...) ...) instead of (cd ...) (...)
;; 5. Broke out keystroke assignments into `my-java-mode-hook'

(defun java-get-package ()
  "Get package definition before point"
  (interactive)
  (save-excursion
    (save-match-data  ;; Added; you want this too
      (goto-char (point-min))
      (when (re-search-forward "^\\s *package\\s +\\(.*\\);"
                               (point-max) t)
        (match-string 1)) )) )

(defun java-package-directory (package)
  "Get package directory of PACKAGE in current buffer"
  (let ((directory (file-name-directory (buffer-file-name)))
        sub-dirs)
    (if directory
        (setq directory (file-truename directory))
      (error "Current buffer is not visiting a file"))
    (save-match-data
      (setq sub-dirs (reverse (split-string package "\\.")))
      (while sub-dirs
        (if (string-match (concat "^\\(.*/\\)"
                                  (regexp-quote (car sub-dirs)) "/$")
                          directory)
            (setq directory (match-string 1 directory)
                  sub-dirs (cdr sub-dirs))
          (error "Package does not match directory structure") ) ) )
    directory) )

(defun java-compile-current-file ()
 "Compiles the current file with javac"
 (interactive)
 (let* ((package (java-get-package))
        (default-directory (java-package-directory package)) )
   (compile
    (concat "javac -Xlint:all " ; Removed too noisy -Werror
        (if package (concat package "/") "")
        (file-name-nondirectory (buffer-file-name)) ) ) ))

(defun java-run-current-file ()
 "Runs the java program the current file corresponds to"
 (interactive)
 (let* ((package (java-get-package))
        (default-directory (java-package-directory package)) )
   (shell-command
    (concat "java "
        (if package (concat package ".") "")
        (file-name-sans-extension
         (file-name-nondirectory (buffer-file-name)) ) )) ))

(defun my-java-mode-hook () "Stuff to run when entering Java mode"
    (local-set-key [(f7)] 'java-run-current-file)
    (local-set-key [(f8)] 'java-compile-current-file) )

(add-hook 'java-mode-hook #'my-java-mode-hook)

The new functions should probably accept a buffer argument so that you can call them on a different buffer than the one you're in, for general usability and adherence to common Emacs conventions. But again, hope this is enough to get you started.