Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add NonEmptyList.reverse #1455

Merged
merged 1 commit into from
Jan 2, 2017
Merged

Conversation

peterneyens
Copy link
Collaborator

No description provided.

@codecov-io
Copy link

codecov-io commented Nov 6, 2016

Current coverage is 92.21% (diff: 100%)

Merging #1455 into master will increase coverage by <.01%

@@             master      #1455   diff @@
==========================================
  Files           242        242          
  Lines          3618       3621     +3   
  Methods        3550       3551     +1   
  Messages          0          0          
  Branches         68         70     +2   
==========================================
+ Hits           3336       3339     +3   
  Misses          282        282          
  Partials          0          0          

Powered by Codecov. Last update 9af39fb...aecda8e

* }}}
*/
def reverse: NonEmptyList[A] =
NonEmptyList.fromListUnsafe(toList.reverse)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it worth implementing this safely (and possibly faster):

  def reverse: NonEmptyList[T] = {

    @annotation.tailrec
    def go(h: T, rest: List[T], acc: List[T]): NonEmptyList[T] =
      rest match {
        case Nil => NonEmptyList(h, acc)
        case h1 :: t1 => go(h1, t1, h :: acc)
      }
    go(head, tail, Nil)
  }

I kind of think so, actually.

@johnynek
Copy link
Contributor

johnynek commented Nov 7, 2016

Looks like the future tests may be flakey on 2.12:

[info] FutureTests:
[info] cats.jvm.tests.FutureTests *** ABORTED *** (41 milliseconds)
[info]   java.util.concurrent.TimeoutException: Futures timed out after [3 seconds]
[info]   at scala.concurrent.impl.Promise$DefaultPromise.ready(Promise.scala:255)
[info]   at scala.concurrent.impl.Promise$DefaultPromise.result(Promise.scala:259)
[info]   at scala.concurrent.Await$.$anonfun$result$1(package.scala:190)
[info]   at scala.concurrent.BlockContext$DefaultBlockContext$.blockOn(BlockContext.scala:53)
[info]   at scala.concurrent.Await$.result(package.scala:123)
[info]   at cats.jvm.tests.FutureTests$$anon$1.eqv(FutureTests.scala:26)
[info]   at cats.jvm.tests.FutureTests$$anon$1.eqv(FutureTests.scala:23)
[info]   at cats.kernel.laws.package$CheckEqOps.$anonfun$$qmark$eq$eq$1(package.scala:23)
[info]   at cats.kernel.laws.package$CheckEqOps.$anonfun$$qmark$eq$eq$1$adapted(package.scala:23)
[info]   at cats.kernel.laws.package$Ops$.run(package.scala:15)
[info]   at cats.kernel.laws.package$CheckEqOps.$qmark$eq$eq(package.scala:23)
[info]   at cats.laws.discipline.package$.catsLawsIsEqToProp(package.scala:10)
[info]   at cats.laws.discipline.MonadTests$$anon$2.props(MonadTests.scala:37)
[info]   at org.typelevel.discipline.Laws$RuleSet.collectParentProps(Laws.scala:80)

@johnynek
Copy link
Contributor

johnynek commented Nov 8, 2016

👍

@johnynek johnynek mentioned this pull request Jan 2, 2017
11 tasks
@fthomas fthomas merged commit 730d521 into typelevel:master Jan 2, 2017
@peterneyens peterneyens deleted the nel-reverse branch January 16, 2017 15:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants