Detect changes in json files regarding dictionary types #38
Conversation
| let removedPropNames = | ||
| getRemovedItems ItemKind.Property flavor | ||
| |> Array.filter (matchInterface dict.Name) | ||
| |> Array.map (fun rp -> rp.Name.Value) |
There was a problem hiding this comment.
I would tack on
|> Set.ofArrayto make the contains check below faster
There was a problem hiding this comment.
Which contains check?
There was a problem hiding this comment.
(fun m -> not (Array.contains m.Name removedPropNames)I would change Array.contains to Set.contains.
There was a problem hiding this comment.
How is Set.contains faster than Array.contains? My impression is the opposite, because at least Set.contains has an implicit conversion from Array to Set.
There was a problem hiding this comment.
Set.contains doesn't do any implicit conversions. You'd pay the construction cost once when building removedPropNames by adding |> Set.ofArray.
Generally, Set.contains is O(lg n) or O(1) and Array.contains is O(n), so if removedPropNames is ever long-ish (10-100 or more items probably?) it will be worth it.
However, I can't find any documentation on Array.contains! Where does Array.contains come from? I see that Shared.fs adds Seq.contains x = Seq.exists ((=)x), which is O(n), but I can't find a definition of Array.contains in our own code.
There was a problem hiding this comment.
Good point. For some reason I was thinking about Seq instead of Set, therefore had the impression that Array.contains is faster. Will update soon.
I believe Array.contains is added in F# 4.0 which was not very well documented.
|
:1+: |
Detect changes in json files regarding dictionary types
Related to microsoft/TypeScript#5773
This fix enables the script to detect added/removed/overridden property for dictionary types in the spec.