Redesign NonEmptyLazyList to be maximally lazy

[NthPortal]

Redesign NonEmptyLazyList to be maximally lazy.

• docs
• tests
• commit/history cleanup?

[danicheg]

LGTM

[satorg]

IMO, this method requires a way more comprehensive scaladocs (preferably with examples).

[NthPortal]

haven't written the docs yet, lol; not gonna do that until the design is solid enough to move forward, so I don't waste effort (docs are hard).

that said, the explanation is fairly simple: fromLazyList and fromLazyListUnsafe are bad, because they evaluate the first element of the LazyList. to remedy that, Maybe allows you to prepend either single elements or NonEmptyLazyLists, guaranteeing that the result is not empty without evaluating any elements

[NthPortal]

trivial usage example:

val nell: NonEmptyLazyList[Int[] = 1 #:: NonEmptyLazyList.maybe(LazyList.empty)

[NthPortal]

better now?

[satorg]

@NthPortal to be honest, I'm still not getting what the purpose those Maybe and Deferred classes are supposed to serve for. IMO by design LazyList is a collection that lazily evaluates its items upon the first traversal. But the instance of LazyList is not supposed to be lazy itself. Therefore it would be nice to have examples that would show how exactly those classes can be useful. And why the same functionality cannot be achieved without them.

[satorg]

Oh, I see that Deferrer is a similar thing to one in the std LazyList, so it seems to be fine then. But Deferrer is just a syntax that is added implicitly and hence transparently for users. Which is not the case for Maybe though.

[NthPortal]

@satorg I'll look into adding an example or two to the docs of maybe

[satorg]

I doubt this method belongs here.
Prepending one LazyList to another one lazily is the job that LazyList.Deferrer seems to be doing already. I.e.:

def a: LazyList[Int] = ???
def b: LazyList[Int] = ???

val maybe1 = NonEmptyList.maybe(a #::: b)
// the above seems to be the same as
val maybe2 = a #::: NonEmptyList.maybe(b)

Anyway a method that only involves LazyList on both sides does not seem to be a part of the NonEmptyLazyList DSL to me.

[NthPortal]

so this question is... tough. conceivably, one might build up a result something like as follows:

def buildResult[A](maybe: NonEmptyLazyList.Maybe[A]): NonEmptyLazyList[A] =
  if (something) someNell #::: maybe
  else if (somethingElse) a #:: b #:: maybe
  else buildResult(someLL #::: maybe)
}
buildResult(NonEmptyLazyList.maybe(LazyList.empty))

now, you could argue that you still don't need it, and can construct the Maybe instance later. which is true. but that seems uglier and less intuitive to me. would love to get thoughts from more people.

Edit: this is example is awful, for reasons that I will explain in another comment below. I'm honestly ashamed to have written it

[satorg]

you could argue that you still don't need it

Yeah, indeed, I would :)

Again just IMO: the entire Maybe does not seem that it should be a part of NonEmptyLazyList per se. Because it looks like a bunch of extension methods for LazyList that simply helps to construct a new NonEmptyLazyList out of it.

For example, see cats.syntax.ListSyntax and its corresponding ListOps.toNel, etc.

So perhaps it would make sense to consider creating a cats.syntax.LazyListSyntax which would provide all this functionality as a bunch of extension methods only.

[NthPortal]

the issue is, NonEmptyLazyList.fromLazyList evaluates the first state/element of the LazyList, which is Bad™; NonEmptyList.fromList does not have this concern as the List is already fully evaluated. the intent behind Maybe was to have a way to construct a NonEmptyLazyList without evaluating any elements. now that fromLazyListUnsafe has been changed to be lazy, Maybe isn't needed as much, though I maintain that it is still a useful tool. its design could perhaps use a bit more workshopping though

[NthPortal]

I've been sick for a week, so I haven't been able to address things, but I'm hopefully better enough to get back to this.

the reason we need some sort of wrapper around LazyList with a #::: operator is that, despite it syntactically being a right-associative prepend operator, #::: actually behaves semantically as a left-associative append operator.

this also means that my example earlier was terrible, and the design of Maybe isn't ideal either. I really have been off my game recently.

I'll re-evaluate and redesign this part of the API to be a bit more reasonable with this in mind

[satorg]

Do we really need this overload with NonEmptyLazyList type if we already have another one that takes LazyList? I am a bit concerned about this overload trick. Usually, overloaded method names are discouraged unless absolutely necessary. But it does not seem to be the case here, because prepending a NonEmptyLazyList can be easily achieved with just the single method above that takes LazyList only:

def a: NonEmptyLasyList[X] = ???
def b: NonEmptyLasyList[X] = ???

val c: NonEmptyLazyList[X] = a.toLazyList #::: b

[NthPortal]

it would be weird to have NonEmptyLazyList be a second-class citizen of its own API. however, the meaning of the methods are identical, yielding no additional conceptual weight and making the API easier to use. additionally, it's not possible to accidentally use the wrong method either, as they are only the same after erasure. I don't personally see any issue with it

[satorg]

But it pretty much corresponds to the way how other APIs are organized in Cats. See for example NonEmptyList.concat vs NonEmptyList.concatNel, also NonEmptyVector.concat vs concatNev, etc. Retrospectively, it would not be a problem to overload concat everywhere in the way as you're proposing. But for some reason the decision was to avoid overloading and provide clearer separate names wherever possible. I personally support it because feel a bit sour about having any sort of "hacks and tricks" in code – they are usually easy to add but tend to turn into maintenance hell in long-term run.

That said, NonEmptyList has the ::: which takes another NonEmptyList, not just List. Perhaps it is just a legacy of that particular class because it was the very first NonEmpty* one. Anyway, it is still not overloaded.

[satorg]

First of all, I have to say that I really appreciate this effort to make NonEmptyLazyList better, thank you @NthPortal for taking on that!

However, let me add my 2 cents worth and outline a few thoughts regarding the APIs being developed. The thoughts may or may not be that good – just my personal perception of the issue:

1. Any syntax sugar (including infix operators like #:::) should not be the primary point of effort. I.e. if there's an operator #::: defined as LL => NELL => NELL, then there should be a corresponding conventional method with a clear name like prependNell or something. In other words, infix operators should be nothing but shortcuts to some conventional counterparts, e.g.

   A #:: LL     <=> LL.prepend(A)
   NELL #::: LL <=> LL.prependNell(NELL)
   

2. When it comes to the #::: operator, I would assume it should get some sort of symmetry. I.e. if A #::: B is possible, then B #::: A should be possible as well. Personally, I see three ways how to organize it.

   • First, it could be defined as a bunch of extension methods on LazyList along with methods defined inside NonEmptyLazyList itself to enable syntax like the following:

     LL   #::: LL   => LL   (already implemented in LL, but has to be re-defined, see below)
     LL   #::: NELL => NELL (can be implemented in NELL)
     NELL #::: LL   => NELL (can be implemented as an extension method to LL)
     NELL #::: NELL => NELL (can be implemented in NELL, has to be overloaded)
     

     (here and below, saying "can be implemented in NELL" I assume NELL.Deferrer of course)
     If to follow this approach we should also get

     A #:: LL => NELL
     

     However, it does not seem right because there's A #:: LL => LL in the ScalaLib already and re-defining the return value could be quite confusing. As well as the fact that we would have to re-define LL #::: LL => LL inside Cats to keep it working once we get NELL #::: LL => NELL into a scope.
   • Another approach could be more specific on types. That way there could be the following syntax enabled:

     LL    #::: LL    => LL   (already implemented in LL, all good)
     W(LL) #::: NELL  => NELL (can be implemented in NELL, takes W as a param)
     NELL  #::: W(LL) => NELL (can be implemented in W created from LL)
     NELL  #::: NELL  => NELL (can be implemented in NELL, takes NELL as a param)
     

     Here W is some sort of wrapper of LL (like the former Maybe / now Tail in the PR's NELL)
     Also, W makes it really easy to support the one-item prepend too:

     A #:: LL    => LL   (already implemented in LL, all good)
     A #:: W(LL) => NELL (can be implemented in W created from LL)
     

     To make lifting of LL to W more convenient it could be enabled as an extension method itself, e.g.

     import cats.syntax.lazyList._
     
     val a: LazyList[Int] = ???
     val b: NonEmptyLazyList[Int] = ???
     val c: NonEmptyLazyList[Int] = a.prepareNell #::: b // requires `a` to be lifted to `W`
     val d: NonEmptyLazyList[Int] = b #::: a.prepareNell // requires `a` to be lifted to `W`
     val e: NonEmptyLazyList[Int] = c #::: d // no lifting required

     where .prepareNell is just the extension method that lifts LL into W.
   • The last approach in my understanding is the strictest one on types. Here, we could avoid mixing-up LL and NELL in the #::: operator and invite a different name for that (##::: maybe?):

     LL   #:::  LL   => LL   (already implemented in LL, all good)
     LL   ##::: NELL => NELL (can be implemented in NELL, takes LL as a param)
     NELL ##::: LL   => NELL (can be implemented as an extension method to LL)
     NELL #:::  NELL => NELL (can be implemented in NELL, takes NELL as a param)
     A    #::   LL   => LL   (already implemented in LL, all good)
     A    ##::  LL   => NELL (can be implemented as an extension method to LL)
     

     Not sure if it is the best way though, but the simplest and lest ambiguous one for sure.

3. Worth to mention that ideally it would be nice if at the end of all we could get such an API for LazyList/NonEmptyLasyList that would correspond the API for the regular List/NonEmptyList, i.e. #:: and #::: could be used for LL/NELL in the same cases as :: and ::: are used for L/NEL. We are not there yet because some separate work is necessary to be done for the regular NonEmptyList, but if we could end up with a good non-conflicting design of APIs for NELL, then later we could add all the missing methods to NEL (in a separate PR of course).

[NthPortal]

there should be a corresponding conventional method with a clear name like prependNell

prepend, prependNell, append and appendNell all already exist

Any syntax sugar (including infix operators like #:::) should not be the primary point of effort

it's rather moot given the above, but regardless, I strongly disagree. despite being symbolic methods, #:: and #::: are the default way of constructing LazyLists; as essentially a specialisation of LazyList, NonEmptyLazyList ought to have an equivalent API so that it is predictable to users of LazyList. LazyList has .unfold on its companion with equivalent power/flexibility as #:: and #:::, but it is less used because it is simply less ergonomic

Here, we could avoid mixing-up LL and NELL in the #::: operator and invite a different name for that

this is possible, but seems unnecessarily confusing and annoying to users. the methods do the same thing, so they should be named the same thing

[NthPortal]

it could be enabled as an extension method itself, e.g. [...] .prepareNell

I really like this idea, though I think something like asNellTail would be a better name