Additions, improvements, and optimizations for StrMap #11
Conversation
| toList :: forall a. StrMap a -> [Tuple String a] | ||
|
|
||
| union :: forall a. StrMap a -> StrMap a -> StrMap a | ||
|
|
||
| unions :: forall a. [StrMap a] -> StrMap a | ||
|
|
||
| unsafeIndex :: forall a. StrMap a -> String -> a |
There was a problem hiding this comment.
The current convention is putting such functions in an Unsafe submodule, though the intro of nullary type classes will change that convention.
… foldMap, Semigroup instance Since we know that all StrMap objects have prototype Object, we know they have no additional enumerable keys, so hasOwnProperty checks are unnecessary. Add efficient JS implementations for keys and values. Add standard Monoid and Monad fold functions, and use them internally. Minor other changes to eliminate unnecessary function layers and variables. Fix and update tests.
ghost
|
|
||
| fromList :: forall a. [Tuple String a] -> StrMap a | ||
| fromList = foldl (\m (Tuple k v) -> insert k v m) empty | ||
| fromList = foldl (\m (Tuple k v) -> _unsafeInsert m k v) (_cloneStrMap empty) |
There was a problem hiding this comment.
Could all these private unsafe helper functions be refactored to use your new ST machinery without affecting performance?
There was a problem hiding this comment.
Actually, some of them, yeah, that's a great idea. Do you have any suggestions for avoiding the module dependency cycle? Put them all in one big module or make an Internal module and re-export?
|
This one is all @paf31, I just did some housekeeping. Looks good to me on the whole though. |
|
@jdegoes If you're happy with all the changes, then it looks good to me, and I'll merge. |
Eliminate some duplicate code in _unsafe helper functions. Move all the ST functions that involve StrMap out of the ST module to do this, and make the ST module interface more directly parallel the STArray one (including an unsafe peek). It would be nice to have more ST operations, but it's not clear the best way to provide them safely without redundant code (the ones that were there before violated Eff semantics).
|
|
Additions, improvements, and optimizations for StrMap
|
Thanks 👍, and great avatar @dylex ;) only just noticed the reference. |
It's unfortunate that the foldM implementation can't properly short-circuit, but otherwise is general enough to support a number of other operations. I've added more efficient JS implementations for select functions, and simplified/generalized some already there.
In a separate commit I've added a basic Data.StrMap.ST mutable interface to JS records. This part probably still needs test cases, so could be left off for now.
All changes should be fully backwards-compatible.