add last, sortBy and sorted to NonEmptyList #1578
Conversation
| def sortBy[B: Order](f: A => B): NonEmptyList[A] = { | ||
| toList.sortBy(f)(Order[B].toOrdering) match { | ||
| case x :: xs => NonEmptyList(x, xs) | ||
| case Nil => sys.error("absurd") |
There was a problem hiding this comment.
I am not particularly proud of this, suggestions welcome!
There was a problem hiding this comment.
This is not really a solution but you can “disable” exhaustivity checking instead:
match {
case List(x, xs @ _*) => NonEmptyList(x, xs)
}I would add a comment, though, to emphasize that we can not get an empty list since we got our list by converting the NonEmptyList into a List
There was a problem hiding this comment.
I think the current thing is fine except I would say: => sys.error("unreachable: reduced the number of items in a NonEmptyList after a sort") or something that is somewhat more of a comment.
There was a problem hiding this comment.
sure, I will update the PR
Codecov Report
@@ Coverage Diff @@
## master #1578 +/- ##
=========================================
- Coverage 92.43% 92.4% -0.04%
=========================================
Files 248 248
Lines 3953 3961 +8
Branches 146 141 -5
=========================================
+ Hits 3654 3660 +6
- Misses 299 301 +2
Continue to review full report at Codecov.
|
| * }}} | ||
| */ | ||
| def sorted[AA >: A](implicit AA: Order[AA]): NonEmptyList[AA] = | ||
| sortBy(a => a: AA) |
There was a problem hiding this comment.
I'd rather not pay the cost to apply the identity function here. Can we instead directly implement?
julien-truffaut
force-pushed
the
last-sort-nel
branch
from
274aa7d
to
4709187
Compare
|
rebased |
| @@ -19,6 +19,8 @@ final case class NonEmptyList[+A](head: A, tail: List[A]) { | |||
| */ | |||
| def toList: List[A] = head :: tail | |||
|
|
|||
| def last: A = tail.lastOption.getOrElse(head) | |||
There was a problem hiding this comment.
Can we instead case match since it is slightly kinder to the GC? The getOrElse allocates a thunk.
| * }}} | ||
| */ | ||
| def sortBy[B: Order](f: A => B): NonEmptyList[A] = | ||
| toList.sortBy(f)(Order[B].toOrdering) match { |
There was a problem hiding this comment.
I think we generally use an implicit parameter (instead of a context bound) when we use the type class instance directly?
There was a problem hiding this comment.
let's be consistent
| * }}} | ||
| */ | ||
| def sorted[AA >: A](implicit AA: Order[AA]): NonEmptyList[AA] = | ||
| toList.sorted(Order[AA].toOrdering) match { |
There was a problem hiding this comment.
Should we do AA.toOrdering?
No description provided.