Improve this numeric solver for properties of geometric objects

Question

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?

• Daniel: 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.
• bradgonesurfing: 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.
• bradgonesurfing: The code compiles fine in tryfsharp.org tryfsharp.org/create/bradgonesurfing/freevars.fsx

Answer 1

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.

• svick: I wonder if it would make sense to go with real class hierarchy and active pattern for the matching.
• pad: If he is more likely to add cases than to add functions processing existing cases, class hierarchy + active patterns is the way to go.