1

i am encountering this problem in F# [not C# where there is already a similar post with a similar answer]

I understand its not possible to modify a Dictionary while enumerating it in a for loop how should i go around that ?

let edgelist1 = [(1,2,3.0f);(1,2,4.0f);(5,6,7.0f);(5,6,8.0f)]
let dict_edges = new Dictionary<int*int,(int*int*float32) list>()
for x in edgelist1 do dict_edges.Add ((fun (a,b,c)-> (a,b)) x, x)
for k in dict_edges.Keys do dict_edges.[k] <- (dict_edges.[k] |> List.rev)

System.InvalidOperationException: Collection was modified; enumeration operation may not execute.

at System.ThrowHelper.ThrowInvalidOperationException(ExceptionResource resource) at System.Collections.Generic.Dictionary`2.KeyCollection.Enumerator.MoveNext() at .$FSI_0101.main@()

individually this is working

dict_edges.[(1,2)] <- dict_edges.[(1,2)] |> List.rev;;

in the for loop i need just to change dictionary values, not keys.

thanks

Fagui Curtain
  • 1,867
  • 2
  • 19
  • 34
  • 1
    Possible duplicate: [How to iterate through Dictionary and change values?](http://stackoverflow.com/questions/2260446/how-to-iterate-through-dictionary-and-change-values) – Guy Coder Apr 22 '16 at 02:52

3 Answers3

5

You can copy all the keys into a temporary list, and then iterate over that list while modifying the original dictionary:

for k in (dict_edges.Keys |> Seq.toList) do 
   dict_edges.[k] <- (dict_edges.[k] |> List.rev)

But I would strongly advise you to rethink your approach and get rid of in-place mutation. This little problem you're facing right now is only the first taste of what could go wrong with a mutation-based program.

Fyodor Soikin
  • 78,590
  • 9
  • 125
  • 172
4

The code you posted is not even syntactically correct, so it's not clear what precisely you are trying to achieve (compiler screams at ((fun (a,b,c)-> (a,b)) x, x) saying that it expects the second x to be a list)

I guess what you are after is: You have a list of weighted edges, where there can be multiple edges between nodes. You'd like to collapse them into a canonical form, where you have all edges grouped that connect any pair of nodes (i,j). Just use any of the groupBy library functions, and you're good:

let map_edges =
    edgelist1
    |> List.groupBy (fun (a, b, _) -> (a, b))
    |> Map.ofList

In the current code you are using ((fun (a,b,c)-> (a,b)) x, x) to extract members of a tuple. Instead, use patterns right in the for expression:

for (a, b, c) in edgelist1 do dict_edges.Add ((a, b), [(a, b, c)])

(I've added [] to make it at least compile)

Note also that you are duplicating information: You store the node tuple in the keys and in the values of the list, making the data structure possibly inconsistent and larger. Consider the following:

let map_edges =
    edgelist1
    |> List.map (fun (a, b, c) -> (a, b), c)
    |> List.groupBy fst
    |> List.map (fun (nodeTuple, edgeList) -> 
        nodeTuple, (edgeList |> List.map snd))
    |> Map.ofList


map_edges
|> Map.iter (fun (nodeI, nodeJ) edgeList ->
    edgeList
    |> Seq.map string
    |> String.concat "; "
    |> printfn "Nodes (%i, %i): weights %s" nodeI nodeJ
)

(You may want to use sequences as the intermediate representation rather than list)

Anton Schwaighofer
  • 3,119
  • 11
  • 24
1

Wouldn't it be easier to just say dict_edges = dict_edges.map( $0.reverse())

Sorry about the bad f# syntax

Andrew Bradnan
  • 36
  • 2