Proper use of fold in F#

Question

I am new to F#. I am trying to use List.fold to help me generate a list of categories and sub-categories based on their Id and ParentId fields. It seems I probably made this code more complex than need be, as I'm getting the stackoverflow error. What am I doing wrong or missing? All related feedback is appreciated.

// types
type CategoryStructure = {
    Id: ValidString;
    ParentId: ValidString;
    Name: ValidString;
    Abbreviation: ValidString;
    Description: ValidString;
    SapId: ValidString;
    Section: ValidString;
    SectionPosition: ValidString
}

type DynamicCategories = {
    Category: CategoryStructure;
    SubCategories: seq<DynamicCategories>
}

// this is the function that produces the stack overflow error
let rec private structureCategories (fullList: CategoryStructure list) 
    (list: CategoryStructure list)  =

    List.fold (fun acc elem -> 


                    // get all categories and details
                    let categories = fullList
                    let mainAcc =
                        [

                            for row in categories do
                                if row = elem
                                then
                                    let subs =  
                                        List.fold (fun acc' elem' ->

                                                    if row.Id = elem'.ParentStructureId
                                                    then

                                                        let foundSubCategory = 
                                                            {
                                                                Category = elem';
                                                                SubCategories = structureCategories fullList list |> Seq.ofList
                                                            }
                                                        foundSubCategory :: acc'
                                                    else acc'

                                                    ) List.empty<DynamicCategories> categories
                                        |> Seq.ofList        
                                    yield{
                                        Category = elem;
                                        SubCategories = subs
                                    }


                        ]

                    mainAcc @ acc
                    ) List.empty<DynamicCategories> list  

// this function gets the initial parent categories and calls the above function
let getStructuredCategories () =

        let categories = allCategoriesAndDetails () |> List.ofSeq
        [
            for row in categories do
                if row.ParentStructureId = NotValid
                then yield row
        ] |> structureCategories categories |> Seq.ofList      

Answer 1

Thanks to Fyodor, I saw my mistake. He was dead on about calling the same arguments. I added this bit of code right before the foundSubCategory value:

let modifiedList = elem' :: List.empty<CategoryStructure>

and then called that value in the subsequent code:

let foundSubCategory = 
        {
            Category = elem';
            SubCategories = structureCategories fullList modifiedList |> Seq.ofList
        }

This solved my issue, but now as Fyodor alluded to, I now have to refactor this into something more efficient.

UPDATE

With the insight that Fyodor pointed out this is the current state of my code, which replaces the original code:

let getStructuredCategories ()  =

    let fullList = allCategoriesAndDetails () 

    let parentList () =
        allCategoriesAndDetails ()
        |> Seq.filter (fun p -> p.ParentStructureId = NotValid)

    let rec toTree (fullList': seq<CategoryStructure>) (parent: CategoryStructure) =

        fullList'
        |> Seq.filter (fun x -> x.ParentStructureId = parent.Id)
        |> Seq.map (fun x -> 
                        {
                            Category = x;
                            SubCategories =
                                toTree fullList' x
                        })
    seq {
        for row in parentList () do
        yield {

            Category = row;
            SubCategories = toTree fullList row    
        }

    }

Answer 2

You keep calling structureCategories with the same arguments - fullList and list. Since arguments are same, it proceeds to do exactly the same thing as on the previous pass, and ends up calling itself again, with the same arguments. And so on.

This is unbounded recursion ("unbounded" here means "doesn't know when to stop recurring"), and it is also not "tail recursion", so quite naturally, it causes stack overflow.

If you want to turn the flat list into a tree-like structure, you could do a bit simpler than this:

let getChildren fullList parentId = fullList |> List.filter (fun c -> c.ParentId = parentId)
let rec toTree fullList root =
  { Category = root;
    SubCategories = 
      getChildren fullList root.Id 
      |> List.map (toTree fullList) }

With this, you'll be left with two problems, which I don't know how to solve without knowing more about your requirements:

1. This will still cause stack overflow if the original list happens to have cycles.
2. You need to decide who the root(s) of the tree is (or are). Intuitively, this would be indicated via "empty" ParentId, but it is unclear from your data structure what "empty" means.

And finally, this naive solution, while better than your original one, is still a bit slower than it needs to be. It iterates over the whole list once, and for every node does another pass to determine its children, resulting in overall complexity of O(N^2). This may be fine if you expect relatively small list, but not so fine for larger lists. In that case, I would first turn the list into a hashtable (keyed by ParentId) and then use that to find children instead of List.filter.