Skip to content

nonemptylist is stack safe - #226

Open
KevinBoyette wants to merge 3 commits into
Kalin-Rudnicki:mainfrom
KevinBoyette:nonemptylist-is-not-stack-safe
Open

nonemptylist is stack safe#226
KevinBoyette wants to merge 3 commits into
Kalin-Rudnicki:mainfrom
KevinBoyette:nonemptylist-is-not-stack-safe

Conversation

@KevinBoyette

Copy link
Copy Markdown
Contributor

No description provided.

Comment on lines +79 to +110
/**
* NOTE: For Kalin
* It's stack safe for iteration, but we can still overflow with really deep nested monadic compositions.
* I've added a trampoline version below for those cases.
*/
def flatMap[B](f: A => NonEmptyList[B]): NonEmptyList[B] = {
// NOTE: For Kalin, I know this isn't idiomatic Scala, but it's stack safe for iteration,
// and I can't think of a better way to do it.
val builder = List.newBuilder[B]
var remaining: List[A] = this.toList

while remaining.nonEmpty do
builder ++= f(remaining.head).toList
remaining = remaining.tail

val result = builder.result()
NonEmptyList(result.head, result.tail)
}

def flatMapTrampoline[B](f: A => TailRec[NonEmptyList[B]]): TailRec[NonEmptyList[B]] = {
def loop(remaining: List[A], acc: List[B]): TailRec[List[B]] =
remaining match
case Nil => done(acc.reverse)
case h :: t =>
tailcall(f(h)).flatMap { nel =>
loop(t, acc.reverse_:::(nel.toList))
}

loop(this.toList, Nil).map { result =>
NonEmptyList(result.head, result.tail)
}
}

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Can you highlight the motivation here?

The purpose of this class is to be a simple wrapper around List[A] where you know you have at least one element. A NonEmptyList will generally get flatMap called on it an average of 0-1 times, maybe 2, and is generally a rather small size (usually single digits).

From my understanding, the purpose of trampolining is for things like cats Effect[_] or zio ZIO[_, _, _], so that they can essentially become your new runtime system and run forever and self recurse without blowing the stack. Thats really not the purpose of this class 😅. Effect/ZIO are like race cars, and this is like a tricycle.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

With needing to to wrap flatMapTrampoline in this TailRec return type, I cant imagine a time where I would ever see normal flatMap and flatMapTrampoline (with extra things to deal with, wrapping/unwrapping TailRec) and say "oh, I need the beefed up version of this for this use case". Sorry, not trying to hate on the fun exercise, but this does not feel like the place for that level of optimization.

Event for the normal flatMap impl, Scala''s List has built-in mutable efficiency under the hood, and you lose that when trying to implement it yourself.

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.

2 participants