2
\$\begingroup\$

I'm converting some C# code to F#. Basically it is some geometry classes that can have properties tuned by a numerical solver. I start with a discriminated union of type parameter that can be fixed or free ( tunable ) and then build tunable geometry objects out of them.

I create a discriminated union called shape with Line, Circle, Point as sub types. The code is below.

module SketchSolveFS.Solver

type parameter =
    | Fixed of double
    | Free of double ref

let free v = Free (ref v)     
let fix v = Fixed v

type point  = {x: parameter; y:parameter}
type line   = {p0: point; p1:point}
type circle = {center: point; radius:parameter}

type shape =
    | Parameter of parameter
    | Point of point
    | Line of line
    | Circle of circle

let rec freeVars v = seq { 
    match v with
    | Parameter p -> 
        match p with
        | Free v -> yield v
        | _ -> ()
    | Point p ->
        yield! Parameter p.x |> freeVars
        yield! Parameter p.y |> freeVars
    | Line line -> 
        yield! Point line.p0 |> freeVars
        yield! Point line.p1 |> freeVars
    | Circle c ->
        yield! Point c.center |> freeVars
        yield! Parameter c.radius |> freeVars
    }

I have a function called freeVars that return me a sequence of tunable parameters in a list of geometry objects. The code works and passes my tests but the implementation leaves me uncomfortable.

Here is why. I keep feeling the need to write my discriminated union this way

type shape =
    Parameter of parameter
    Point of Parameter * Parameter
    Line of Point * Point

instead of having to create concrete types before hand. Then my function freeVars would look like this.

let rec freeVars v = seq { 
    match v with
    | Parameter p -> 
        match p with
        | Free v -> yield v
        | _ -> ()
    | Point p ->
        yield! p.x |> freeVars
        yield! p.y |> freeVars
    | Line line -> 
        yield! line.p0 |> freeVars
        yield! line.p1 |> freeVars
    | Circle c ->
        yield! c.center |> freeVars
        yield! c.radius |> freeVars
    }

where I don't have to wrap the concrete classes back in the discriminated union wrappers each time I extract and recursively process. Now obviously the F# compiler doesn't like this or I wouldn't be asking the question.

Is there a better way to frame the working code. Are discriminated unions not appropriate for this code perhaps?

\$\endgroup\$

migrated from stackoverflow.com Jun 11 '13 at 19:42

This question came from our site for professional and enthusiast programmers.

  • \$\begingroup\$ There are a lot of things for the compiler not to like. Do some basic clean-up (correct caps, supply correct parameter types, etc) and re-post with a specific question. \$\endgroup\$ – Daniel Jun 11 '13 at 19:12
  • \$\begingroup\$ I think you missed the point. The first bit of code compiles fine. It just doesn't seem quite right. The second bit of code is rubbish but tries to indicate that I want to write a recursive discriminated union that refers to specific subtypes but it is not allowed. I can't quite figure out why it should not be allowed or if there is a better way to achieve the same thing. \$\endgroup\$ – bradgonesurfing Jun 11 '13 at 19:17
  • \$\begingroup\$ The code compiles fine in tryfsharp.org tryfsharp.org/create/bradgonesurfing/freevars.fsx \$\endgroup\$ – bradgonesurfing Jun 11 '13 at 19:23
3
\$\begingroup\$

A few suggestions on the first code fragment:

  • Change type names to upper case
  • Change record field names to upper case
  • Change fields of line to Begin and End to be descriptive
  • Use pattern matching on records instead of . access

The updated code fragment is as follows:

type Parameter =
    | Fixed of double
    | Free of double ref

let free v = Free (ref v)     
let fix v = Fixed v

type Point  = { X: Parameter; Y: Parameter }
type Line   = { Begin: Point; End: Point }
type Circle = { Center: Point; Radius: Parameter }

type Shape =
    | Parameter of Parameter
    | Point of Point
    | Line of Line
    | Circle of Circle

let rec freeVars v = seq { 
    match v with
    | Parameter p -> 
        match p with
        | Free v -> yield v
        | _ -> ()
    | Point {X = x; Y = y} ->
        yield! Parameter x |> freeVars
        yield! Parameter y |> freeVars
    | Line {Begin = p0; End = p1} -> 
        yield! Point p0 |> freeVars
        yield! Point p1 |> freeVars
    | Circle {Center = c; Radius = r} ->
        yield! Point c |> freeVars
        yield! Parameter r |> freeVars
    }

And here are comments on the second code fragment.

You mistakenly assume that Line, Circle, Point, etc as sub types of shape. They are just union cases, so your type definition doesn't make sense.

You can change shape to be recursive:

type shape =
    | Parameter of parameter
    | Point of shape * shape
    | Line of shape * shape

but it allows more cases than it should.

You can use tuples instead of records:

type shape =
    | Parameter of parameter
    | Point of parameter * parameter
    | Line of (parameter * parameter) * (parameter * parameter)
    | Circle of (parameter * parameter) * parameter

but it is less desirable since Point, Line and Circle deserve to be types on their own rights.

Are discriminated unions not appropriate for this code perhaps?

It depends. If you have fixed number of cases, then DUs and pattern matching are great to use. Otherwise, you have to fall back to class hierarchy which is much more painful to deal with.

\$\endgroup\$
  • \$\begingroup\$ I wonder if it would make sense to go with real class hierarchy and active pattern for the matching. \$\endgroup\$ – svick Jun 11 '13 at 20:56
  • \$\begingroup\$ If he is more likely to add cases than to add functions processing existing cases, class hierarchy + active patterns is the way to go. \$\endgroup\$ – pad Jun 12 '13 at 7:44

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.