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

4.1.4.3 Shaping Up 2 (Da Streets) exercise's solution gives wrong result #79

Open
ibanezang opened this issue Jul 23, 2019 · 2 comments

Comments

@ibanezang
Copy link

I just want to give feedback regarding the "4.1.4.3 Shaping Up 2 (Da Streets)" exercise.
The answer to that exercise gives the wrong result.

sealed trait Rectangular {
  def width: Double
  def height: Double
  val sides = 4
  val perimeter = 2*width + 2*height
  val area = width*height
}

case class Square(size: Double) extends Rectangular {
  val width = size
  val height = size
}

case class Rectangle(
                      val width: Double,
                      val height: Double
                    ) extends Rectangular


val square = Square(10)
square.perimeter
square.area
square.sides

val rectangle = Rectangle(5, 6)
rectangle.perimeter
rectangle.area
rectangle.sides

I found out that the square's perimeter and area are always returning 0.0
to fix it I changed width and height into def as below.

case class Square(size: Double) extends Rectangular {
  def width = size
  def height = size
}
@ibanezang ibanezang changed the title 4.1.4.3 Shaping Up 2 (Da Streets) exercise solution gives wrong result 4.1.4.3 Shaping Up 2 (Da Streets) exercise's solution gives wrong result Jul 23, 2019
@zeshansali
Copy link

I just came across this as well and was wondering if anyone knows why using val results in broken perimeter and area methods?

@davegurnell
Copy link
Contributor

Thanks for this issue, all. We'll work out a way to include it in the book text eventually, I promise!

I'm not 100% sure on all of the details of why this particular case fails, but it'll definitely be due to Scala's initialisation order. Here's what I do know.

When an object is created, the bodies of the classes and traits in its hierarchy are executed in order from supertype to subtype. This includes any statements in the body of the class and the RHS of any val.

The classic bug you see is when the RHS of a val refers to a later val that hasn't been initialised yet:

class Foo {
  val a = b
  val b = "Bar"
}

In this example, the RHS of a is executed before b has been initialised, resulting in a == null. There are checks in place in the compiler (especially in recent versions of Scala) to prevent simple cases like this one, but I expect things like inheritance and overriding vals with other vals will confuse them (they certainly confuse me).

I tend to prefer not to define abstract vals unless there's some special case that requires them. It's better to declare abstract defs and override them with a def, a val, or a lazy val as seems fit:

trait Foo {
  def bar: String
}

// You can override a def with a val:
class Baz extends Foo {
  val bar = "Qux"
}

// Alternative to the above. A case class field is effectively a val:
case class Baz(bar: String) extends Foo

In general, the pattern you should use for abstract data types is:

  • Define a sealed trait or sealed abstract class for the sum type. Feel free to add defs to its body but don't add vals or lazy vals.

  • Extend the sum type with one or more final case classes or case objects.

  • Don't declare vals in the sum type. This could cause problems like those above.

  • Don't extend the case classes further. This could cause a range of other problems.

I hope this helps!

Cheers,
Dave

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

No branches or pull requests

3 participants