Critique of immutable classes with circular references design, and better options

Question

I have a factory class that creates objects with circular references. I'd like them to be immutable (in some sense of the word) too. So I use the following technique, using a closure of sorts:

[<AbstractClass>]
type Parent() =
  abstract Children : seq<Child>
and Child(parent) =
  member __.Parent = parent

module Factory =

  let makeParent() =
    let children = ResizeArray()
    let parent = 
      { new Parent() with
        member __.Children = Seq.readonly children }
    [Child(parent); Child(parent); Child(parent)] |> children.AddRange
    parent

I like this better than an internal AddChild method because there's a stronger guarantee of immutability. Perhaps it's neurotic, but I prefer closures for access control.

Are there any pitfalls to this design? Are there better, perhaps less cumbersome, ways to do this?

Not an answer, but `[Child(parent); Child(parent); Child(parent)] |> List.iter children.Add` could be reduced to `[Child(parent); Child(parent); Child(parent)] |> children.AddRange`. Personally, I use the approach you're demonstrating a lot when using `ReadOnlyCollection<>`. — ildjarn, Aug 11 '11 at 19:59
I don't know why, `ReadOnlyCollection` feels hacky to me. Throwing an exception when calling `Add` isn't quite the behavior I'd like. I'd rather the `Add` method not be there. — Daniel, Aug 11 '11 at 20:01
I don't recall ever needing it in F#, but in C# there's no good built-in alternative. In any case, the same could be said of F#'s `dict`. :-] — ildjarn, Aug 11 '11 at 20:03
Good point. Although `dict` returning an `IDictionary<'K, 'V>` likely has more to do with compatibility than the lack of good alternatives. — Daniel, Aug 11 '11 at 20:04
I think the same can be said of `ReadOnlyCollection<'T>`, given that it implements `IList<'T>`. In any case, we digress... ;-] — ildjarn, Aug 11 '11 at 20:13
Is the abstractness of `Parent` necessary? If true immutability is important, you can use records to create cycles in immutable object graphs (this can get a bit ugly, though). — kvb, Aug 11 '11 at 20:29
@kvb: No. It's abstract merely to facilitate the closure. I'm curious about how to do this with records, although, in my case it won't work because `Parent` implements an interface. (Correction: just discovered records can implement interfaces--that's news to me). — Daniel, Aug 11 '11 at 20:31
@ildjarn,@daniel: doesn't `Map<'k,'v>` count as a built-in alternative to `dict`? — pblasucci, Aug 12 '11 at 13:03
@pblasucci : They have different semantics. `Map<>` is immutable because it returns a new `Map<>` when you call `Add`, `Remove`, etc., leaving the original unchanged; `dict` returns an `IDictionary<>` which is immutable because it throws when you attempt to alter it by calling `Add`, `Remove`, etc. — ildjarn, Aug 12 '11 at 15:54
@Daniel - note that you don't actually need the AbstractClass with object expression for this solution to work: you can make `Parent` a normal class with a constructor taking `Child seq` and pass in `Seq.readonly children` since `children` is mutable and `Seq.readonly` will use its `Enumerator` under-the-hood. — Stephen Swensen, Aug 12 '11 at 16:50
@Stephen: Good point. I think that's an artifact of a prior design. — Daniel, Aug 12 '11 at 18:11
@ildjarn: I see your point; they are conceptually different. — pblasucci, Aug 12 '11 at 20:52

score 9 · Accepted Answer · edited Aug 11 '11 at 21:28

You can use F#'s support for recursive initialization even when creating an instance of abstract class:

let makeParent() =
  let rec children = seq [ Child(parent); Child(parent); Child(parent) ]
  and parent = 
    { new Parent() with
      member __.Children = children }
  parent

When compiling the code, F# uses lazy values, so the value children becomes a lazy value and the property Children accesses the value of this lazy computation. This is fine, because it can first create instance of Parent (referencing the lazy value) and then actually construct the sequence.

Doing the same thing with records wouldn't work as nicely, because none of the computations would be delayed, but it works quite nicely here, because the sequence is not actually accessed when creating the Parent (if it was a record, this would be a field that would have to be evaluated).

The F# compiler cannot tell (in general) whether this is correct, so it emits a warning that can be disabled using #nowarn "40".

In general, I think that using let rec .. and .. to initialize recursive values is a good thing - it is a bit limited (one of the references must be delayed), but it forces you to keep the recursive references isolated and, I think, it keeps your code simpler.

EDIT To add an example when this may go wrong - if the constructor of Child tries to access the Children collection of its parent, then it forces evaluation of the lazy value before it can be created and you get a runtime error (which is what the warning says). Try adding this to the constructor of Child:

do printfn "%d" (Seq.length parent.Children)

That's impressive. Should I be concerned about the resulting warning: `This and other recursive references to the object(s) being defined will be checked for initialization-soundness at runtime through the use of a delayed reference. This is because you are defining one or more recursive objects, rather than recursive functions.`? — Daniel, Aug 11 '11 at 20:47
@Daniel - added a few words about it. I wouldn't worry about this - usually, the warning means that you used it for the purpose for which it was designed :-). — Tomas Petricek, Aug 11 '11 at 20:58
Seeing design considerations like this increases my appreciation for the F# team. — Daniel, Aug 11 '11 at 21:05

score 4 · Answer 2 · answered Aug 11 '11 at 21:57

I think that Tomas's answer is the way to go. However, for completeness I'll show how you could use recursive records to create cyclic immutable objects. This can potentially get quite ugly, so I've hidden the immutable record implementation behind some nicer properties:

type Parent = internal { children : Children option }
and internal Children = { first : Child; rest : Children option }
and Child = internal { parent : Parent }

let rec internal listToChildren = function
| [] -> None
| c::cs -> Some { first = c; rest = listToChildren cs }

let rec internal childrenToList = function
| None -> []
| Some { first = c; rest = cs } -> c::(childrenToList cs)

module Factory =
    let makeParent() = 
        let rec parent = { children = children }
        and child1 = { parent = parent }
        and child2 = { parent = parent }
        and child3 = { parent = parent }
        and children = [child1; child2; child3] |> listToChildren
        parent

type Parent with
    member p.Children = childrenToList p.children
type Child with
    member c.Parent = c.parent

Wow. That's weird. Thanks for spelling it out, although, I'm not sure I totally understand it. — Daniel, Aug 11 '11 at 22:26
@kvb - won't `childrenToList p.children` be calculated on every call to `Parent` member `Children`? — Stephen Swensen, Aug 12 '11 at 14:56

score 0 · Answer 3 · answered Aug 12 '11 at 05:16

I guess something like this can also be done:

type ParentFactory private (n) as X = 
    inherit Parent()
    let childs = [for i=1 to n do yield Child(X :> Parent)]
    override X.Children = childs |> List.toSeq;               
    static member Create n = (new ParentFactory(n)) :> Parent

Critique of immutable classes with circular references design, and better options

3 Answers3

Linked