Replace Data.Map with Data.Map.Strict #4675
Conversation
Jimbo4350
requested review from
newhoggy,
deepfire,
jutaro,
coot,
mgmeier,
fmaste,
cleverca22,
dcoutts,
erikd,
JaredCorduan,
LudvikGalois and
CarlosLopezDeLara
as code owners
| import Data.Map (Map) | ||
| import qualified Data.Map as Map | ||
| import Data.Map.Strict (Map) | ||
| import qualified Data.Map.Strict as Map | ||
| import qualified Data.Map.Strict as SMap |
There was a problem hiding this comment.
We are importing Data.Map.Strict twice here. Also wondering if we should import as SMap to prevent confusion.
There was a problem hiding this comment.
I would actually just suggest replacing all uses of SMap with Map.
There was a problem hiding this comment.
So LMap if we ever need to reach for a lazy map?
There was a problem hiding this comment.
If it ever comes up, personally, I'd suggest importing Data.Map.Lazy as Lazy unless that clashes with something, because it reads rather nicely.
The underlying datatype is shared between Data.Map.Lazy and Data.Map.Strict, so simply using Data.Map.Strict isn't a magic bullet, since something innocuous like fmap won't keep your values in WHNF and you can easily be given "lazy" maps where the values aren't already in WHNF. Honestly, I think it'd be better off as a newtype if you "really" want strictness, but that's probably be a discussion for another time.
There was a problem hiding this comment.
I think Lazy is too generic (both Text and ByetString have lazy versions. I also think LMap is not as good a name as LazyMap.
There was a problem hiding this comment.
I'm changing it to Map in this PR as it's concerned with the removal of Data.Map. However John and Robert can have the ultimate decision (whilst considering Erik's input) and open a new PR with the relevant changes. This PR should also update the style doc so we can compile a list of how modules should be imported so it's not ambiguous.
Jimbo4350
force-pushed
the
jordan/remove-data-map-usage
branch
from
73745b0
to
063b245
Compare
|
bors r+ |
4675: Replace Data.Map with Data.Map.Strict r=Jimbo4350 a=Jimbo4350 Resolves #4648 Co-authored-by: Jordan Millar <[email protected]>
|
Build failed: |
Jimbo4350
force-pushed
the
jordan/remove-data-map-usage
branch
from
063b245
to
57545bc
Compare
Jimbo4350
force-pushed
the
jordan/remove-data-map-usage
branch
from
57545bc
to
6194bca
Compare
Resolves #4648