Proposal: Rename HashMap.lookupDefault to HashMap.findWithDefault

[Matt Renaud]

> I'm wondering if we should ultimately get rid of that function
altogether, from both packages

I'm on the fence about this one since its such a common operation, I
personally would be a little surprised if it wasn't part of the API. You're
right that it's not /that/ much code to write, but I imagine everyone will
end up writing their own findWithDefault in their codebase, I may be wrong
though.


On Wed, Jan 24, 2018 at 4:27 PM, David Feuer <david.feuer at gmail.com> wrote:

> I'm wondering if we should ultimately get rid of that function altogether,
> from both packages. From an API design standpoint, it's kind of silly,
> because
>
>     findWithDefault d k m =
>        fromMaybe d (lookup k m)
>
> At present, there's a slight performance penalty to doing it that way. I
> wonder if we can squash that now that GHC has unboxed sums.
>
>     lookup# :: ... => k -> m a -> (# (# #) | a #)
>     lookup k m = case lookup# k m of
>       (# _ | #) -> Nothing
>       (# | a #) -> Just a
>
> Now case-of-case will get rid of the extra Maybe.
>
>
> On Jan 23, 2018 9:15 PM, "Matt Renaud" <matt at m-renaud.com> wrote:
>
> *What*
>
> *====*
>
> Rename unordered-container's Data.HashMap.lookupDefault
> <https://hackage.haskell.org/package/unordered-containers-0.2.8.0/docs/Data-HashMap-Strict.html#v:lookupDefault> to
> findWithDefault.
>
> findWithDefault :: (Eq k, Hashable k) => v -> k -> HashMap k v -> v
>
> Note: There are no functionality changes, this is purely a rename.
>
>
> *Why*
> *===*
>
> This aligns the Data.HashMap API with containers' Data.Map.findWithDefault
> <https://hackage.haskell.org/package/containers-0.5.11.0/docs/Data-Map-Strict.html#v:findWithDefault>
> .
>
> Data.Map.findWithDefault :: Ord k => a -> k -> Map k a -> a
>
> The map APIs provided by the two packages are *almost* drop in
> replacement compatible if you aren't using any of the implementation
> specific functions (like ordering based functions from Data.Map), this
> change brings us one step closer.
>
> API consistency reduces the cognitive overhead when learning a new
> package. Having to learn different function names for the same
> functionality depending on which "map" implementation you're using is a
> poor developer experience.
>
> We chose the containers' name findWithDefault over unordered-containers'
> lookupDefault for two reasons:
>
>    1. Existing lookupX functions returns a Maybe value, while findX
>    functions return a non-wrapped value.
>    2. The containers package ships with GHC and is a "core" package.
>
>
> *Pros:*
> *-----*
>
> - Consistent API between different "map" implementations (Data.Map,
> Data.IntMap, Data.HashMap). This makes switching implementations an import
> change.
> - Naming matches other similar functions (lookupX return Maybe-wrapped
> values)
>
> *Cons:*
> *-----*
>
> - API change requires users to update their code
>   + unordered-containers has A LOT of users: 358815 total (13325 in the
> last 30 days)
>
>
> *How*
> *===*
>
> https://github.com/tibbe/unordered-containers/pull/176/commi
> ts/152f8818ee13dacb370e49b904edc4c1a4c8f87b
>
> *Code Changes:*
> *-------------*
>
> - Rename the function in Data.HashMap.Base (and expose it from Strict and
> Lazy modules)
> - Make lookupDefault an INLINE alias of findWithDefault
> - Add DEPRECATION notice to lookupDefault
> - Bump unordered-containers version to 0.2.9.0
>
>
> *Migration - Option 1:*
>
> *---------------------*
>
> *- *Announce on Haskell communication channels (haskell-cafe@,
> haskell-community@, #haskell on Twitter, Reddit thread, etc.)
> - Users of unordered-containers >= 0.2.9.0 receive warning about
> deprecated function
> - Code can be updated by find and replace: s/lookupDefault/findWithDefaul
> t/
> - lookupDefault with deprecation notice remains for 1 year (subject to
> change)
> - after 1 year the lookupDefault function is removed, unordered-containers
> version bumped to 0.3.0.0 (major version bump due to breaking change)
>
> *Migration - Option 2:*
>
> *---------------------*
>
> - Announce on Haskell communication channels (haskell-cafe@,
> haskell-community@, #haskell on Twitter, Reddit thread, etc.)
> - Users of unordered-containers >= 0.2.9.0 receive warning about
> deprecated function
> - Code can be updated by find and replace: s/lookupDefault/findWithDefaul
> t/
> - lookupDefault function is never removed
>
>
> *Discussion: *
> *===========*
>
> I would like to get some comments on this proposal. In particular:
> - Is the small API churn worth the increase in consistency?
> - Should migration option 1 (completely remove the old function) or 2
> (keep old function indefinitely) be taken? We can punt on this and go with
> option 2 to start and revisit later if desired.
>
> I hope a decision about the proposal can be reached by 2018-02-09. Thanks!
>
>
>
>
>
> _______________________________________________
> Libraries mailing list
> Libraries at haskell.org
> http://mail.haskell.org/cgi-bin/mailman/listinfo/libraries
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.haskell.org/pipermail/libraries/attachments/20180129/345c8a7a/attachment.html>