0
\$\begingroup\$

I've recently begun using Clojure, and I wanted to translate a project that I wrote with p5.js to us Clojure and quil. Unfortunately, the code resulting from a direct translation is bad enough to make a grown man cry. Here is the original js code for reference: (you'll need it :)

var xOffset = 450 / 2, yOffset = 450 / 2;
var zoom = 1;

var trace = true;

var functionString = "x";

function setup() {
    createCanvas(450, 450);

};

function draw() {

    background(255);

    // println(scale);
    strokeWeight(1);
    stroke(128);
    // draw grid
    for (x = 0; x < width / 50 + 2; x++) {
        line(x * 50 + xOffset % 50, 0, x * 50 + xOffset % 50, height);
    }

    for (y = 0; y < height / 50 + 2; y++) {
        line(0, y * 50 + yOffset % 50, width, y * 50 + yOffset % 50);
    }

    // draw the thick line
    stroke(0);
    strokeWeight(4);
    if (xOffset / 2 <= width / 2)
        line(xOffset, 0, xOffset, height);

    if (yOffset / 2 <= height / 2)
        line(0, yOffset, width, yOffset);

    stroke(0, 197, 255);
    strokeWeight(2);
    var lastVector = createVector(0, 0);

    var amtPerPix = zoom / 50;

    // draw the line
    for (x = -1; x < width; x = x + 1) {
        var y = getYforX((x - xOffset) * amtPerPix) / amtPerPix + yOffset;

        line(x, y, lastVector.x, lastVector.y);
        lastVector.x = x;
        lastVector.y = y;
        if (x === mouseX && trace)
            ellipse(x, y, 5, 5);
    }

    textFont("Helvetica");
    textSize(12);
    textAlign(constants.LEFT, constants.TOP);
    strokeWeight(0.5);

    // draw the text

    stroke(0);
    for (x = -1; x < width / 50 + 1; x++) {
        var word = (Math.ceil(Math.floor(-xOffset) / 50) + x) * zoom;
        var xloc = x * 50 + xOffset % 50;
        var yloc = constrain(yOffset, textWidth(word) + 22, height);

        push();
        translate(xloc + 1, yloc - 1);
        rotate(-PI / 2);
        text(word, 0, 0);
        pop();
    }
    for (y = -1; y < width / 50 + 1; y++) {
        var word = (Math.ceil(Math.floor(-yOffset) / 50) + y) * zoom;

        var xloc = constrain(xOffset, 1, width - textWidth(word) - 2);
        var yloc = y * 50 + yOffset % 50;
        text(word, xloc + 1, yloc + 1);
    }

    var word = functionString;
    if (trace) {
        var x = (mouseX - xOffset) * amtPerPix;
        line(mouseX, 0, mouseX, height);
    }

    if (mouseIsPressed) {
        xOffset -= pmouseX - mouseX;
        yOffset -= pmouseY - mouseY;
    }
};
function getYforX(x) {
    return x;
};

And this is my new code: (the mapping isn't one to one since I added some features to the cljs version, most notably the update-zoom fn)

(ns project-graph.proj-graph-main
  (:require [quil.core :as q :include-macros true]))

(enable-console-print!)


(def zoom-settings [[0.125 45] [0.25 60] [0.25 30] [0.5 70] [0.5 40] [1 70] [1 50] [1 30] [2 50] [2 30] [4 40] [8 65]])

(def canvas-size [450 450])

(defonce graph-state (atom {:offset     (map #(/ % 2) canvas-size)
                            :trace      true
                            :zoom-level 1
                            :grid-size 30
                            :zoom-setting-index 7
                            :func       #(* 6 (.sin js/Math (/ % 5)))}))


(defn setup []
  (q/frame-rate 15)
  (q/background 255))

(defn draw-number [axis spaces-from-edge grid-offset-x grid-offset-y]
  (let [base [grid-offset-x grid-offset-y]
        [moving-cord const-cord] (if (= axis :vert) base (reverse base))
        txt (.floor js/Math (- const-cord))
        txt (+ spaces-from-edge (.ceil js/Math (/ txt (:grid-size @graph-state))))
        txt (* txt (:zoom-level @graph-state))

        const-cord (+ (* (:grid-size @graph-state) spaces-from-edge) (mod const-cord (:grid-size @graph-state)))
        moving-cord (if (= axis :horiz)
                      (q/constrain (+ -4 grid-offset-y) (+ 2 (q/text-width txt)) (q/height))
                      (q/constrain (+ 4 grid-offset-x) 3 (- (q/width) (q/text-width txt) 2)))
        x-cord (if (= axis :horiz) const-cord moving-cord)
        y-cord (if (= axis :horiz) moving-cord const-cord)]
    (q/push-matrix)
    (q/push-style)
    (q/fill 0)
    (q/stroke 0)
    (q/translate x-cord y-cord)
    (when (= axis :horiz)
      (q/rotate (/ q/PI -2)))
    (q/text txt 0 0)
    (q/pop-style)
    (q/pop-matrix)))

(defn draw []
  (q/background 255)
  (q/stroke-weight 1)
  (q/stroke 0 0 0 20)

  ;grid
  (let [grid-size (:grid-size @graph-state)
        [grid-offset-x grid-offset-y] (:offset @graph-state)
        [grid-offset-x-mod grid-offset-y-mod] (map #(mod % grid-size) (:offset @graph-state))]
    ; x axis drawing
    (dorun (for [x (range (.round js/Math (/ (q/width) grid-size)))]
             (do (q/line (+ grid-offset-x-mod (* x grid-size)) 0 (+ grid-offset-x-mod (* x grid-size)) (q/height))
                 (draw-number :horiz x grid-offset-x grid-offset-y))))
    (dorun (for [y (range (.round js/Math (/ (q/height) grid-size)))]
             (do (q/line 0 (+ grid-offset-y-mod (* y grid-size)) (q/width) (+ grid-offset-y-mod (* y grid-size)))
                 (draw-number :vert y grid-offset-x grid-offset-y))))

    ;zeros
    (q/stroke 0)
    (q/stroke-weight 2)
    (q/line grid-offset-x 0 grid-offset-x (q/height))
    (q/line 0 grid-offset-y (q/width) grid-offset-y)

    ; draw the line itself
    (q/stroke 231 76 60)
    (let [amtPerPix (/ (:zoom-level @graph-state) grid-size)]
      (reduce (fn [[lx ly] cx]
                (let [y (+ grid-offset-y (/ (->> (- cx grid-offset-x)
                                                 (* amtPerPix)
                                                 ((:func @graph-state)))
                                            amtPerPix))]
                  (when (and (:trace @graph-state) (= cx (q/mouse-x)))
                    (q/ellipse cx y 5 5))
                  (q/line cx y lx ly)
                  [cx y]))
              [0 0] (range -10 (+ 1 (q/width)))))

    (when (:trace @graph-state)
      (q/line (q/mouse-x) 0 (q/mouse-x) (q/height)))


    )


  )

(defn update-offset []
  (let [move-x (- (q/mouse-x) (q/pmouse-x))
        move-y (- (q/mouse-y) (q/pmouse-y))
        curr-offset (:offset @graph-state)]
    (swap! graph-state assoc :offset [(+ (first curr-offset) move-x) (+ (second curr-offset) move-y)])))

(defn update-zoom [scroll-amt]
  (let [curr-idx (:zoom-setting-index @graph-state)
        next-idx (q/constrain (+ curr-idx scroll-amt) 0 (- (count zoom-settings) 1))
        old-zoom (* (:zoom-level @graph-state) (:grid-size @graph-state))
        [new-zoom new-grid-size] (nth zoom-settings next-idx)
        half-window-size (map #(/ % 2) canvas-size)
        new-graph-center (map + (map #(/ (* % old-zoom) (* new-zoom (:grid-size @graph-state)))
                                     (map - (:offset @graph-state) half-window-size))
                              half-window-size)
        change-setting! (partial swap! graph-state assoc)]
    (change-setting! :zoom-level new-zoom)
    (change-setting! :grid-size new-grid-size)
    (change-setting! :zoom-setting-index next-idx)
    (change-setting! :offset new-graph-center)))

(defn init-ui! []
  (q/defsketch graph                                      
               :title "Graphing calculator"             
               :host "quil-canvas"
               :settings #(q/smooth 2)                      
               :setup setup                                 
               :draw draw                                   
               :size canvas-size
               :mouse-dragged update-offset
               :mouse-wheel update-zoom))

So.... Yeah, it stinks. First of all, the code repition:

(dorun (for [x (range (.round js/Math (/ (q/width) grid-size)))]
         (do (q/line (+ grid-offset-x-mod (* x grid-size)) 0 (+ grid-offset-x-mod (* x grid-size)) (q/height))
             (draw-number :horiz x grid-offset-x grid-offset-y))))
(dorun (for [y (range (.round js/Math (/ (q/height) grid-size)))]
         (do (q/line 0 (+ grid-offset-y-mod (* y grid-size)) (q/width) (+ grid-offset-y-mod (* y grid-size)))
             (draw-number :vert y grid-offset-x grid-offset-y))))

Both the dorun calls have nearly the exact same code, but I can't think of any concise way to roll it all into one function. To see the other major pain point, go checkout draw-number. The let binding in particular.

  (let [base [grid-offset-x grid-offset-y]
        [moving-cord const-cord] (if (= axis :vert) base (reverse base))
        txt (.floor js/Math (- const-cord))
        txt (+ spaces-from-edge (.ceil js/Math (/ txt (:grid-size @graph-state))))
        txt (* txt (:zoom-level @graph-state))

        const-cord (+ (* (:grid-size @graph-state) spaces-from-edge) (mod const-cord (:grid-size @graph-state)))
        moving-cord (if (= axis :horiz)
                      (q/constrain (+ -4 grid-offset-y) (+ 2 (q/text-width txt)) (q/height))
                      (q/constrain (+ 4 grid-offset-x) 3 (- (q/width) (q/text-width txt) 2)))
        x-cord (if (= axis :horiz) const-cord moving-cord)
        y-cord (if (= axis :horiz) moving-cord const-cord)]

The only way I've accomplished even half-way decent math readability is through continuous rebinding. Specifically for this, can I perform some sort of infix math in ClojureScirpt?

What are the best ways to address the problems mentioned above, or any other ugly bits that arn't idiomatic Clojure?

\$\endgroup\$
2
\$\begingroup\$

The dorun/for combo is excessive. When carrying out side effects over a list, use doseq:

(doseq [x (range (.round js/Math (/ (q/width) grid-size)))]
  (q/line (+ grid-offset-x-mod (* x grid-size)) 0 (+ grid-offset-x-mod (* x grid-size)) (q/height))
  (draw-number :horiz x grid-offset-x grid-offset-y))

That allows you to shorten the first line by a bit, and eliminate the do.


I tried reducing the 2 loops down into a function, but for it to be sane, I had to push a lot of the computation outside the function, which itself became very messy. An easy way to neaten it up though is to create an alias for .round js/Math. It can even be a macro so there's no runtime cost:

(defmacro roundM [expr]
  `(.round js/Math ^double ~expr))

(doseq [x (range (roundM (/ (q/width) grid-size)))]
  (q/line (+ grid-offset-x-mod (* x grid-size)) 0 (+ grid-offset-x-mod (* x grid-size)) (q/height))
  (draw-number :horiz x grid-offset-x grid-offset-y))

and if ceiling instead of rounding is acceptable, you can drop the rounding altogether:

(doseq [x (range (/ (q/width) grid-size)))]
    ...)

When I have a long call to q/line, I've found having one x/y pair per line helps readability:

(doseq [x (range (roundM (/ (q/width) grid-size)))]
  (q/line (+ grid-offset-x-mod (* x grid-size)) 0
          (+ grid-offset-x-mod (* x grid-size)) (q/height))
  (draw-number :horiz x grid-offset-x grid-offset-y))

I also prefer this since I use Cursive's Parinfer, and "stacking" code like this helps it manage braces better.

\$\endgroup\$
  • \$\begingroup\$ Thanks for the tips! I'll try your suggestions soon. By the way, I too use cursive, and like it very much. \$\endgroup\$ – J Atkin Apr 7 '17 at 22:07
  • \$\begingroup\$ Np. And ya, Cursive is great. The fact that IntelliJ+Cursive is free is amazing. Great environment to write in. \$\endgroup\$ – Carcigenicate Apr 7 '17 at 22:12

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.