Skip to content

Revise groupAllBy to just use an Ordering function #189

Closed
@milesfrain

Description

@milesfrain

Proposing we modify the newly-added (in #182) groupAllBy from:

groupAllBy :: forall a. Ord a => (a -> a -> Boolean) -> List a -> List (NEL.NonEmptyList a)

to:

groupAllBy :: forall a. (a -> a -> Ordering) -> List a -> List (NEL.NonEmptyList a)

The existing version can lead to undesired output where the Ord instance fights against your clustering rule.
The new version also more closely matches the pattern set by these other functions:

sortBy :: forall a. (a -> a -> Ordering) -> List a -> List a
-- (the below is true after #179)
nubBy :: forall a. (a -> a -> Ordering) -> List a -> List a

If we do this before 0.14, we don't even have to worry about an API bump.

@kl0tl for review.

Metadata

Metadata

Assignees

Labels

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions