Skip to content

Conversation

satorg
Copy link
Contributor

@satorg satorg commented Jul 14, 2024

Adds a code generator for Gen.zipWith functions.

Example:

case class Foo(pos: Int, ascii: String, hex: String)

Gen.zipWith(Gen.posNum[Int], Gen.asciiStr, Gen.hexStr)(Foo.apply)

Without Gen.zipWith the same was possible to accomplish with Gen.zip, however it would required an additional map over an intermediary tuple instance:

Gen.zip(Gen.posNum[Int], Gen.asciiStr, Gen.hexStr)
  .map { case (pos, ascii, hex) => Foo(pos, ascii, hex)) }

Therefore Gen.zipWith allows to avoid the intermediate conversion which can come in handy in come cases.

Note that Gen.zipWith starts with arity 2 whereas Gen.zip starts with arity 1.
However, I think the latter was an oversight – Gen.zip for arity 1 is a no-op function.

@rossabaker
Copy link
Member

Is this intended to behave similarly to mapN, but without the Cats dependency?

@satorg
Copy link
Contributor Author

satorg commented Aug 13, 2024

Is this intended to behave similarly to mapN, but without the Cats dependency?

Pretty much. To be more accurate, it is supposed to resemble functions map2 through map22 of the Apply typeclass, because mapN requires a tuple to be created first, check this out:

case class Foo(pos: Int, ascii: String, hex: String)

val gen1 = (Gen.posNum[Int], Gen.asciiStr, Gen.hexStr).mapN { Foo.apply }

val gen2 = Apply[Gen].map3(Gen.posNum[Int], Gen.asciiStr, Gen.hexStr) { Foo.apply }

val gen3 = Gen.zipWith(Gen.posNum[Int], Gen.asciiStr, Gen.hexStr) { Foo.apply }

However, there's one catch with the Cats functions: they both use a chain of Functor.product under the hood, which in turn calls to Gen.zip with arity 2. I.e. the Cats functions incur a bunch of intermediary tuples on every evaluation to be created.

@satorg satorg force-pushed the gen-zip-with branch 2 times, most recently from e206580 to cad9980 Compare September 1, 2024 01:39
@satorg satorg requested a review from Copilot June 12, 2025 18:20
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This pull request introduces a multi-arity generator method, Gen.zipWith, which lets users combine multiple generators and apply a mapping function directly without creating an intermediary tuple.

  • Adds a new zipWith method implementation in codegen.scala that constructs a chain of flatMap/map calls based on the supplied arity.
  • Updates tests in GenSpecification.scala to validate zipWith for various arities, from 2 to 22.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
project/codegen.scala Introduces the zipWith generator, constructing composed generator expressions via flatMap and map calls.
core/jvm/src/test/scala/org/scalacheck/GenSpecification.scala Adds property tests for zipWith with multiple arities to ensure correctness.
Comments suppressed due to low confidence (1)

project/codegen.scala:120

  • [nitpick] Consider adding an inline comment to clarify the purpose and structure of destructuring the last element of tTtgs; this can help others understand the foldRight initializer logic.
val ((_, ti), gi) = tTtgs.last

Copy link
Member

@rossabaker rossabaker left a comment

Choose a reason for hiding this comment

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

I lost track of this one in a GitHub bankruptcy. I'm sorry it got so desperately lonely you had to talk to a bot.

@satorg
Copy link
Contributor Author

satorg commented Jun 15, 2025

@rossabaker , thank you for the review!

I'm sorry it got so desperately lonely you had to talk to a bot.

No worries! Actually, I just wanted to check out that "copilot review" feature – to see how it works, what the output looks like, etc. So I thought – what could be better than some my own PR for that?

@satorg satorg merged commit d817094 into typelevel:main Jun 18, 2025
18 checks passed
@satorg satorg deleted the gen-zip-with branch June 18, 2025 00:18
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