4

I'm trying to create a Gen for BST with ScalaCheck, but when I call .sample method it gives me java.lang.NullPointerException. Where do I wrong?

sealed trait Tree

case class Node(left: Tree, right: Tree, v: Int) extends Tree

case object Leaf extends Tree

import org.scalacheck._
import Gen._
import Arbitrary.arbitrary

case class GenerateBST() {

  def genValue(left: Tree, right: Tree): Gen[Int] = (left, right) match {
    case (Node(_, _, min), Node(_, _, max)) => arbitrary[Int].suchThat(x => x > min && x < max)
    case (Node(_, _, min), Leaf) => arbitrary[Int].suchThat(x => x > min)
    case (Leaf, Node(_, _, max)) => arbitrary[Int].suchThat(x => x < max)
    case (Leaf, Leaf) => arbitrary[Int]
  }


  val genNode = for {
    left <- genTree
    right <- genTree
    v <- genValue(left, right)
  } yield Node(left, right, v)

  def genTree: Gen[Tree] = oneOf(const(Leaf), genNode)
}

GenerateBST().genTree.sample
Martijn Pieters
  • 1,048,767
  • 296
  • 4,058
  • 3,343
Marco Mantovani
  • 111
  • 2
  • 7

1 Answers1

3

Because of the way you're defining the generator recursively for a recursive data type, you need to use Gen.lzy at the top:

def genTree: Gen[Tree] = Gen.lzy(oneOf(const(Leaf), genNode))

As an unrelated side note, using suchThat in your generator definitions should generally only be a last resort. It means that sample will often fail (about a third of the time with the fixed version of your code), and more importantly that if you someday want to create arbitrary functions resulting in Tree, you're going to see a lot of horrible org.scalacheck.Gen$RetrievalError: couldn't generate value exceptions.

In this case you can avoid suchThat pretty easily by using Gen.chooseNum and swapping the left and right sides if they're in the wrong ordered:

sealed trait Tree
case class Node(left: Tree, right: Tree, v: Int) extends Tree
case object Leaf extends Tree

import org.scalacheck.{ Arbitrary, Gen }

object GenerateBST {
  def swapIfNeeded(l: Tree, r: Tree): (Tree, Tree) = (l, r) match {
    // If the two trees don't have space between them, we bump one and recheck:
    case (Node(_, _, x), n @ Node(_, _, y)) if math.abs(x - y) <= 1 =>
      swapIfNeeded(l, n.copy(v = y + 1))
    // If the values are in the wrong order, swap:
    case (Node(_, _, x), Node(_, _, y)) if x > y => (r, l)
    // Otherwise do nothing:
    case (_, _) => (l, r)
  }

  def genValue(left: Tree, right: Tree): Gen[Int] = (left, right) match {
    case (Node(_, _, min), Node(_, _, max)) => Gen.chooseNum(min + 1, max - 1)
    case (Node(_, _, min), Leaf) => Gen.chooseNum(min + 1, Int.MaxValue)
    case (Leaf, Node(_, _, max)) => Gen.chooseNum(Int.MinValue, max - 1)
    case (Leaf, Leaf) => Arbitrary.arbitrary[Int]
  }

  val genNode = for {
    l0 <- genTree
    r0 <- genTree
    (left, right) = swapIfNeeded(l0, r0)
    v <- genValue(left, right)
  } yield Node(left, right, v)

  def genTree: Gen[Tree] = Gen.lzy(Gen.oneOf(Gen.const(Leaf), genNode))
}

Now you can use Arbitrary[Whatever => Tree] without worrying about generator failures:

scala> implicit val arbTree: Arbitrary[Tree] = Arbitrary(GenerateBST.genTree)
arbTree: org.scalacheck.Arbitrary[Tree] = org.scalacheck.ArbitraryLowPriority$$anon$1@606abb0e

scala> val f = Arbitrary.arbitrary[Int => Tree].sample.get
f: Int => Tree = org.scalacheck.GenArities$$Lambda$7109/289518656@13eefeaf

scala> f(1)
res0: Tree = Leaf

scala> f(2)
res1: Tree = Node(Leaf,Leaf,-20313200)

scala> f(3)
res2: Tree = Leaf

scala> f(4)
res3: Tree = Node(Node(Leaf,Leaf,-850041807),Leaf,-1)
Travis Brown
  • 138,631
  • 12
  • 375
  • 680
  • Hi, how I have this exception ! Exception raised on property evaluation. > Exception: org.scalacheck.Gen$Choose$IllegalBoundsError: invalid bounds: low=1, high=-1 org.scalacheck.Gen$Choose$$anon$5.choose(Gen.scala:383) org.scalacheck.Gen$Choose$$anon$5.choose(Gen.scala:381) org.scalacheck.Gen$Choose$$anon$7.choose(Gen.scala:423) org.scalacheck.Gen$.chooseNum(Gen.scala:822) $line6.$read$$iw$$iw$GenerateBST$.genValue(:21) and I don't understand why… Can you help me? – Marco Mantovani Mar 01 '19 at 16:33
  • 1
    Oh, right, if they're the same you'll have to bump one (instead of swapping). I'll update the answer in a second. – Travis Brown Mar 01 '19 at 16:37
  • Okay, the range issue is fixed. – Travis Brown Mar 01 '19 at 16:47
  • You might still want to put some kind of check on depth, since `genNode` could get called repeatedly (but that's something you'd want with either the `suchThat` version or mine). – Travis Brown Mar 01 '19 at 16:49
  • ok, but in this way we don't always create a correct BST because when we bump we don't check if the change involves right child. If the right child value is one more than the father, we don't have a BST. Am I right? – Marco Mantovani Mar 01 '19 at 17:06
  • @MarcoMantovani That's why we `swapIfNeeded` again after the bump. – Travis Brown Mar 01 '19 at 17:11
  • sorry, I badly explained myself. If **l0 <- genTree** create **((.1.)3(.2.))** and **l1 <- genTree** create **((.5.)4(.6.))** we have that the BTS created is **(((.1.)3(.2.))4((.5.)5(.6.)))**. Maybe I make some error in my idea. – Marco Mantovani Mar 01 '19 at 17:16
  • @MarcoMantovani This probably deserves a new question, since the original one is about the `lzy` issue and the `suchThat` part is unrelated. – Travis Brown Mar 01 '19 at 17:41