c-cube/ocaml-containers

Inconsistent meaning of map2 in CCPair

kwshi opened this issue · 2 comments

kwshi commented

In the list-like modules CCList, CCArray, CCString, etc., map2 has the signature (a -> b -> c) -> a t -> b t -> c t, corresponding to a zip-with operation: zip two lists together, combining elements component-wise using the given function (a -> b -> c).

In CCPair, the map2 function has a completely different meaning: it has signature (b -> c) -> a * b -> a * c, corresponding to a map-the-second-element operation. The discrepancy between the meanings of map2 is a bit confusing: when reaching for this sort of function, my first instinct is always to write map_fst or map_snd (since those names sound much more like they're describing what I want--namely mapping the first/second element, preserving structure), and never map1 or map2, since those remind me of the zip-with functions instead.

Since this issue is an API/naming issue, any fix for it would (unfortunately) introduce backwards-incompatible API changes. Acknowledging that, here are my suggestions for how I think the naming would make more sense:

  • map_fst : (a -> c) -> a * b -> c * b, map_snd : (b -> c) -> a * b -> a * c.
    • The names fit the functionality more aptly. "map" functions typically preserve the structure of a container (e.g. 'a list -> 'b list, string -> string), rather than extracting individual parts of a data structure (e.g. snd : a * b -> b). Thus the old behavior, which is just fst/snd composed with a function directly--no structure preserved, only extraction--does not fit the name "map" that well, whereas modifying a part of a tuple while preserving its structure/shape does fit the name "map".
    • The old behavior can be achieved just as easily (and in fact more concisely) using fst %> and snd %>, which read more clearly to me as well.
  • Remove map1.
  • If we're interested in a map2 function, I'd mimic the signature of the existing map generalized to zipping (hence analogous to the *2 naming in other modules):
    • map2 : (a1 -> b1 -> c1) -> (a2 -> b2 -> c2) -> a1 * a2 -> b1 * b2 -> c1 * c2 implemented as map2 f g (a1, a2) (b1, b2) = (f a1 b1, g a2 b2).
    • and maybe even a map_same2 : (a -> b -> c) -> a * a -> b * b -> c * c = fun f -> map2 f f
kwshi commented

A similar thing with CCResult.map2, which I feel could be more aptly named map_both.

I think it makes sense to not use map2 for something inconsistent with List.map2 and the likes, indeed. I agree map_snd makes more sense (resp map_fst for map1).

A map2 function that corresponds to zip would be useful indeed. However, I'd argue the current map_fst is nice to have, except it's maybe better described as… fst_then_map 😅