2
\$\begingroup\$

I often do golf challenges, and thought it might be advantageous to write some code that automatically removes all the unnecessary whitespace from a program.

After a few refactors, this is what I ended up with. In all the tests I've run it through, it's correctly removed only the whitespace that doesn't change the meaning of the program.

Example:

(minimize-code (fn [a] (+ a 1)))

"(fn[a](+ a 1))"

Limitations:

  • It doesn't handle line-comments well. It doesn't remove comments, so anything after the first ; is commented out.

  • It only removes unnecessary whitespace. Names still need to be manually shortened (maybe a later project).

What I what advice on:

  • Really, I'm looking for any advice. It actually ended up being much simpler than I anticipated, and I'm fairly happy with the end result. I'm sure there's room for improvement though!

(ns bits.golf.helpers.minimizer)

(def safe-neighbors
  "The characters that indicate neighboring spaces are safe to remove."
  (set "(){}[]\" \t\n"))

(def minimizer-output-path "./minimized-code.clj")

(defn can-remove-cur-char?
  "Returns whether or not a character is safe to remove.
   A character is considered unsafe to remove if it doesn't change the meaning of the program.
   Expects that if prev-char or next-char aren't available, nil will be passed."
  [prev-char cur-char next-char]
  (cond
    (not (Character/isWhitespace ^Character cur-char))
    false

    (or (nil? prev-char) (nil? next-char))
    true

    :else
    (or (safe-neighbors prev-char) (safe-neighbors next-char))))

(defn safe-remove-whitespace
  "Removes any whitespace characters that don't effect the meaning of the program."
  [code-str]
  (loop [[chr & rest-str] code-str
         acc ""]
    (if-not chr
      acc
      (let [prev-chr (if (empty? acc) nil (last acc))
            next-chr (if rest-str (first rest-str) nil)
            can-remove? (can-remove-cur-char? prev-chr chr next-chr)
            replacement-chr (if (Character/isWhitespace ^Character chr)
                              \space chr)]
        (recur rest-str
               (if can-remove? acc (str acc replacement-chr)))))))

(defn minimize-code*
  "Removes unnecessary whitespace and outputs it to minimizer-output-path, then returns the minimized code."
  [code-str]
  (let [minimized-code (safe-remove-whitespace code-str)]
    (spit minimizer-output-path minimized-code)
    minimized-code))

(defmacro minimize-code
  "Removes unneccesary whitespace and outputs it to minimizer-output-path, then returns the minimized code."
  [& body]
  (minimize-code* (apply str body)))
\$\endgroup\$
4
  • \$\begingroup\$ With this kind of problem where you only need to look at the neighbours I believe it is possible to partition 2 1 then map rather than use recursion. I'd be happy to try out or see that approach... \$\endgroup\$ – Chris Murphy Feb 28 '17 at 2:15
  • \$\begingroup\$ @ChrisMurphy I originally used partition, but it doesn't give enough context. If many spaces exist in a row, it incorrectly removed all of them regardless of what came before the spaces, since it only sees the immediate previous neighbour instead of the previous neighbor that wasn't removed. If you have a version that uses partition though, I'd like to see it. \$\endgroup\$ – Carcigenicate Feb 28 '17 at 2:21
  • \$\begingroup\$ Would a previous step (as in step in a ->> macro) that used reduce to chomp all whitespace down to one space character do the trick, or is it more complex than that? \$\endgroup\$ – Chris Murphy Feb 28 '17 at 2:34
  • \$\begingroup\$ @ChrisMurphy I'll have to think on it. Thanks for the thoughts. \$\endgroup\$ – Carcigenicate Feb 28 '17 at 2:40
1
\$\begingroup\$

Instead of both writing to a file and returning a string from minimize-code* I would separate the writing to a file part into a new helper.

(defmacro minimize-to-file [& body]
  `(spit ~minimizer-output-path (minimize-code ~@body)))

Or

(defn to-file [s]
  (spit minimizer-output-path s))

The current behavior is quite surprising, even for a tiny util that you're only going to run in the repl.

Also there's no need to worry about comments. Your minimize-code* function doesn't handle them by itself, but comments are stripped out by the macro before it is seen by the function.

(defmacro foo [& body]
  (apply str body))

(foo
 (a-fn ;; a comment
  an-arg))
;=> "(a-fn an-arg)"

Edit:

This also means that (partition 2 1 ,,, like @Cris Murphy suggested in the comments will probably work, as whitespace is also collapsed by being read by the macro.

\$\endgroup\$
1
\$\begingroup\$

Here's something that gets the same answer using different techniques:

(def safe-neighbors
  (set "(){}[]\" \t\n"))

(defn whitesp? [ch]
  (Character/isWhitespace ^Character ch))

(defn del-white [s]
  (reduce #(if (and (whitesp? %2)
                    (whitesp? (last %1)))
             %1
             (str %1 %2)) "" s))

(defn squeezable? [[a b c]]
  (and (whitesp? b) (or (safe-neighbors a) (safe-neighbors c))))

(defn elide [triples]
  (let [squeezabilities (drop-last (cons true (map (complement squeezable?) triples)))]
    (->> (map vector squeezabilities triples)
         (filter first)
         (map (comp first second)))))

(defn squeeze-safe [s]
  (apply str (apply str (->> s (partition 3 1) elide)) (take-last 2 s)))

(defn minimize-code-2* [code-str]
  (->> code-str
       del-white
       squeeze-safe))

(defmacro minimize-code [& body]
  (minimize-code-2* (apply str body)))

(defn x-1 []
  (minimize-code (fn [a]    (+ a   1))))

In particular it removes recursion from being the crux of the code. Loop/recur recursion can solve most problems, but can quickly become liable to frailty and difficult to reason about as more and more loop variables are added.

Here the starting off point was to think that partition can be used to look at the neighbouring characters by only grabbing three characters at a time. Thus partition can replace recursion. You know exactly what is going on with partition, however recursion is less focused, takes more time to comprehend and is prone to atrophy.

Here removing white-space and seeing if the final squeeze(*) can be done are two different operations, done on different parses through the input. Thus when writing (and reading) the code you can concentrate on one simple thing at a time.

Here squeezable? can be replaced by can-remove-cur-char?, as the latter is almost certainly more well thought out than my effort!

(*) Squeeze as in squeeze out the unnecessary white-space between neighbouring characters

\$\endgroup\$
2
  • \$\begingroup\$ Welcome to StackExchange Code Review! Please see: How do I write a good answer?, where you will find: "Every answer must make at least one insightful observation about the code in the question. Answers that merely provide an alternate solution with no explanation or justification do not constitute valid Code Review answers and may be deleted". \$\endgroup\$ – Stephen Rauch Feb 28 '17 at 6:12
  • \$\begingroup\$ Thanks for a different perspective. My primary concern was a review of my code though, so I'm going to keep it open. \$\endgroup\$ – Carcigenicate Feb 28 '17 at 19:20

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.