purescript-arrays icon indicating copy to clipboard operation
purescript-arrays copied to clipboard

Consistency for Eq a / Ord a functions with suffix "By"

Open Trequetrum opened this issue 5 years ago • 3 comments

As I look through the API, I see a pretty consistent pattern of "You can rely on a typeclass for Eq/Ord or you can supply your own".

So if a type signature has forall a. Eq a => ..., there's often a version that removes this constraint and adds in a function like forall a. (a -> a -> Boolean) ...

There even seems like a pretty consistent naming pattern:

insert    & insertBy 
sort      & sortBy
group     & groupBy
groupAll  & groupAllBy
nub       & nubBy
nubEq     & nubByEq  <- Sort of a break from the pattern, but still
union     & unionBy
delete    & deleteBy
intersect & intersectBy

But it feels sort of arbitrary when some functions don't follow that pattern

For example: difference

I'd expect to see

difference :: forall a. Eq a => Array a -> Array a -> Array a
differenceBy :: forall a. (a -> a -> Boolean) -> Array a -> Array a -> Array a

The same holds for elem, notElem, elemIndex, and elemLastIndex, but these are covered by find >>> isJust, find >>> isNothing, findIndex, and findLastIndex. I understand a departure in naming pattern here since Eq a is replaced with (a -> Boolean) instead of (a -> a -> Boolean).

Trequetrum avatar Apr 30 '21 20:04 Trequetrum

I agree with this idea on principle. However, it does mean making breaking changes.

JordanMartinez avatar May 02 '21 23:05 JordanMartinez

At least for difference and differenceBy, this wouldn't be a breaking change.

differenceBy doesn't exist yet.

Trequetrum avatar May 03 '21 13:05 Trequetrum

I'd actually like to consider removing the Eq-based difference and replacing it with an Ord one, because the Eq one's performance is terrible. So that's a breaking change anyway. Adding a differenceBy sounds fine to me too. I'd prefer to leave elem, notElem, elemIndex, and elemLastIndex as they are though, personally.

hdgarrood avatar May 03 '21 18:05 hdgarrood