5
\$\begingroup\$

I am new to Haskell, and have been reading Learn You a Haskell for Great Good!.

I've rewritten a problem that I recently solved in JavaScript in Haskell to practice what I have been reading.

A little bit of a back story with out rambling on too much; I wanted to create a circle using bezier curve drawing commands. Similar to SVG but for an Android vector drawable. The reason I needed the circle in a path command is for animation. I was using AnimatedVectorDrawable to animate a circle into a square. If your drawable has the same path with the same number of commands in two different files it can animate fluidly between them. Making a square with curve commands is easy enough, but the circle needed some math.

What I did to quickly generate my circle path was create a script in Javascript to do it. Being a math problem I felt this was a good candidate for Haskell exercise.

Here is my Haskell script:

-- Usage:
-- beziercircle [circumfrance] [offest x] [offset y]
--
-- Example: 
-- beziercircle 500
--
--
-- Calculates a circle using bezier paths for using in android
-- vector drawables
--
import System.Environment
import Text.Printf

bz = 0.552284749831
zero = read "0" :: Float

main = do
    args <- getArgs

    let c = parseArg 0 args
    let d = c / 2
    let ox = parseArg 1 args
    let oy = parseArg 2 args

    let ps = points d ox oy
    let cs = controls zero (ip d) (op d) c ox oy

    putStrLn $ (showMove (zero + ox, d + oy)) ++ (showAllCurves ps cs) ++ "Z"

    where
        parseArg i args = (if length args >= i + 1 then read (args !! i) :: Float else zero)

ip d = d - (d * bz)
op d = d + (d * bz)

controls a b c d x y = map (\(a, b, c, d) -> (ox a,oy b,ox c,oy d)) [c1, c2, c3, c4]
    where   ox = (+x)
            oy = (+y)
            c1 = (a, b, b, a)
            c2 = (c, a, d, b)
            c3 = (d, c, c, d)
            c4 = (b, d, a, c)


points d x y = map (\(x,y) -> (ox x, oy y)) [p2, p3, p4, p1]
    where   ox = (+x)
            oy = (+y)
            p1 = (zero, d)
            p2 = rotate90 p1
            p3 = offset d p2
            p4 = offset d p1

offset o (x, y) = (x + o, y + o)

rotate90 (x, y) = (y, x * (-1))

showMove (x, y) = printf "M %f %f \n" x y

showCurve (x, y) (cx1, cy1, cx2, cy2) = do
    printf "C %f %f %f %f %f %f \n" cx1 cy1 cx2 cy2 x y

showAllCurves as bs = concat $ zipWith (showCurve) as bs

The script is based on this question How to create circle with Bezier Curves

Being new I am sure there are lots of places I can make this script better. I am hoping to learn from feedback! I know it can be commented more, mostly looking for ways to shrink the code using techniques that may not be familiar to me. Even if there is a Math equation that I am no seeing, that can solve this better. That would be great to learn as well!

Here is my original Javascript (Node.js) for reference as well:

// Creates a Vector path command for a circle based on based in circumfrence

var args = process.argv.slice(2);

if (args.length < 1) {
    console.log("Please supply a width for the vector circle path");
}

// The second and third argument can be used to offset the circle by X adn Y number of pixels
var offsetXBy = 0;
var offsetYBy = 0;
if (args.length >= 2) {
    offsetXBy = parseFloat(args[1]);
}

if (args.length >= 3) {
    offsetYBy = parseFloat(args[2]);
}


function Coord(x, y) {
    var self = this;

    this.x = parseFloat(x);
    this.y = parseFloat(y);

    this.offset = function(offsetX, offsetY) {
        self.x += offsetX;
        self.y += offsetY;
    }
}

Coord.prototype.toString = function () {
    return `${this.x} ${this.y}`
}

function BezierCurve(x, y, c1, c2) {
    var self = this;

    this.x = x;
    this.y = y;
    this.c1 = c1;
    this.c2 = c2;

    this.offset = function(offsetX, offsetY) {
        self.x += offsetX;
        self.y += offsetY;

        self.c1.offset(offsetX, offsetY);
        self.c2.offset(offsetX, offsetY);
    }
}

BezierCurve.prototype.toString = function () {
    return `${this.c1.toString()} ${this.c2.toString()} ${this.x} ${this.y}`
}

var BEZIER_CONTROL_POINT = 0.552284749831;

var dimension = parseFloat(args[0]); // Circles circumfrence
var halfDimen = dimension / 2;
var controlPointOffset = halfDimen * BEZIER_CONTROL_POINT;


// from Middle Left
var firstMove = new Coord(0, halfDimen);

// curve to Top Middle
var curve1 = new BezierCurve(halfDimen, 0, 
                    new Coord(0, halfDimen - controlPointOffset),
                    new Coord(halfDimen - controlPointOffset, 0));

// curve to Middle Right
var curve2 = new BezierCurve(dimension, halfDimen,
                    new Coord(halfDimen + controlPointOffset, 0),
                    new Coord(dimension, halfDimen - controlPointOffset));

// curve to Bottom Middle
var curve3 = new BezierCurve(halfDimen, dimension,
                    new Coord(dimension, halfDimen + controlPointOffset),
                    new Coord(halfDimen + controlPointOffset, dimension));

// curve back to Middle Left
var curve4 = new BezierCurve(0, halfDimen,
                    new Coord(halfDimen - controlPointOffset, dimension),
                    new Coord(0, halfDimen + controlPointOffset));

if (offsetXBy > 0) {

    firstMove.offset(offsetXBy, offsetYBy);
    curve1.offset(offsetXBy, offsetYBy);
    curve2.offset(offsetXBy, offsetYBy);
    curve3.offset(offsetXBy, offsetYBy);
    curve4.offset(offsetXBy, offsetYBy);
}


console.log(`M ${firstMove.toString()}`);
console.log(`C ${curve1.toString()}`);
console.log(`C ${curve2.toString()}`);
console.log(`C ${curve3.toString()}`);
console.log(`C ${curve4.toString()}`);
console.log("Z");
\$\endgroup\$
  • 1
    \$\begingroup\$ Welcome to Code Review! I have rolled back the last edit. Please see what you may and may not do after receiving answers. \$\endgroup\$ – Mathieu Guindon Sep 16 '16 at 15:56
  • \$\begingroup\$ I will be posting revisions here: github.com/davethomas11/codereview_Q_141491 \$\endgroup\$ – Dave Thomas Sep 16 '16 at 16:26
  • \$\begingroup\$ @Mat'sMug Would it be appropriate to add comments explaining the code? I'd like to make it clear what the variables are and what the functions are doing. I won't edit the main original code, but add comments after the code block explaining it. \$\endgroup\$ – Dave Thomas Sep 16 '16 at 17:33
  • \$\begingroup\$ IMO that shouldn't be necessary - comments are part of what's reviewable, someone could start posting an answer addressing the redundancy/wordiness of the comments and suggest using more meaningful identifiers instead ;-) \$\endgroup\$ – Mathieu Guindon Sep 16 '16 at 17:35
  • \$\begingroup\$ Ok I will explain the variables in question to Peter in the comments under his answer. =) \$\endgroup\$ – Dave Thomas Sep 16 '16 at 17:39
2
\$\begingroup\$

Let's start with the back story:

I wanted to create a circle using bezier curve drawing commands. Similar to SVG but for an Android vector drawable. The reason I needed the circle in a path command is for animation.

The documentation for Android's VectorDrawable says that it uses exactly the same path syntax as SVG. That means that it supports A and a for circular arcs. Have you actually tested that they don't work?


The variable names are not as helpful as they could be. What is bz? Given that one of the inputs is documented as circumfrance (correct spelling is circumference), I can't understand why I don't see pi anywhere: it is actually the diameter instead?

Edit: having compiled and tested the code, it's definitely the diameter rather than the circumference, so the comment is buggy.


SVG paths have relative and absolute versions of the movement instructions. If you were to use the relative c instead of the absolute C you could eliminate the offsets, simplifying the code.

Edit: to justify my claim that using relative movement rather than absolute simplifies the code, here's the full program using relative movement, minus the header comment:

import System.Environment
import Text.Printf

bz = 0.552284749831

main = do
    args <- getArgs

    let d = parseArg 0 args
    let r = d / 2
    let ox = parseArg 1 args
    let oy = parseArg 2 args

    putStrLn $ (showMove (ox, r + oy)) ++ (showAllQuadrants r) ++ "Z"

    where
        parseArg i args = (if length args >= i + 1 then read (args !! i) :: Float else 0.0)

quadrant r = [(0.0, -bz*r), ((1.0-bz)*r, -r), (r, -r)]

quadrants r = [q1, q2, q3, q4]
    where   rotate90AC (x, y) = (-y, x)
            q1 = quadrant r
            q2 = map rotate90AC q1
            q3 = map rotate90AC q2
            q4 = map rotate90AC q3

showMove (x, y) = printf "M %f %f \n" x y

showQuadrant [(cx1, cy1), (cx2, cy2), (x, y)] = do
    printf "c %f %f %f %f %f %f \n" cx1 cy1 cx2 cy2 x y

showAllQuadrants r = concat $ (map showQuadrant (quadrants r))

I'm sure quadrants can be simplified further by someone who knows the Haskell standard library better than me.


You say that you're looking for techniques to shrink the code. The simplest one is to not overcomplicate. Consider

controls a b c d x y = map (\(a, b, c, d) -> (ox a,oy b,ox c,oy d)) [c1, c2, c3, c4]
    where   ox = (+x)
            oy = (+y)

vs

controls a b c d x y = map (\(a, b, c, d) -> (a+x, b+y, c+x, d+y)) [c1, c2, c3, c4]

I find the second easier to read as well as shorter.

\$\endgroup\$
  • \$\begingroup\$ I am using C command instead of A (arch) command because the second shape I am animating too is a square. I am able to create a straight line with a C command. I will research and see if I can create a straight line with an A command. \$\endgroup\$ – Dave Thomas Sep 16 '16 at 13:58
  • \$\begingroup\$ Agreed, good advice, variable names can be clearer. I am going to do an edit to clarify the code's meaning. \$\endgroup\$ – Dave Thomas Sep 16 '16 at 14:00
  • \$\begingroup\$ Using the relative c command would require same number of coordinates. It would also change the logic. All points used must be relative to last commands ending position. It wouldn't be any simpler, just another method. \$\endgroup\$ – Dave Thomas Sep 16 '16 at 14:03
  • \$\begingroup\$ The function that assigns the control points is easier to read the way you wrote it. I will make that change. \$\endgroup\$ – Dave Thomas Sep 16 '16 at 14:05
  • \$\begingroup\$ Update: The A (arch) command does don't give me the behaviour that I want. Four curved lines becoming four straight lines works best with the C command. \$\endgroup\$ – Dave Thomas Sep 16 '16 at 17:30

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.