Gabriel95 / scalafx

Automatically exported from code.google.com/p/scalafx
Other
0 stars 0 forks source link

Setting control effect as null causes NPE #91

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago
What steps will reproduce the problem?
1. Create a Node, ie,

val drop = new Rectangle {
  width = 300
  height = 300
}

2. Asign an effect to the control, ie,

drop.effect = new DropShadow {
  radius = 5.0
  offsetX = 3.0
  offsetY = 3.0
  color = Color.CORAL
}

3. Set effect to null when action triggers
drop onDragExited = { e:DragEvent =>
  drop.effect = null
}

What is the expected output? What do you see instead?
No effect. NPE.

What version of the product are you using? On what operating system?
ScalaFX M5, JDK 8b105, Sbt 0.13, Scala 2.10.2, Windows 7 64bits

Please provide any additional information below.
Adding support for Optional Types would make this easier and more idiomatic, 
but it would need to solve the use cases

drop.effect = new Effect { ... }
drop.effect = None
drop.effect = null //quite picky

Original issue reported on code.google.com by dae...@gmail.com on 24 Sep 2013 at 3:35

GoogleCodeExporter commented 8 years ago
A quick note
case class Effect(val id:Int) extends AnyVal
object Node {
  def effect(e:Effect) = println(e) //XXX NPE when null feeded
  def effect(e:Option[Effect]) = println(e getOrElse "none provided") //XXX NPE when null feeded
}

Node.effect( Effect(2) )
Node.effect( None )
Node.effect( null ) //not allowed at compile-time

At runtime, semantically, we would expect a NPE as a uncontrolled case and an 
Exception should be thrown, because when developers does not want the attribte 
they should use None.

Original comment by dae...@gmail.com on 24 Sep 2013 at 3:48

GoogleCodeExporter commented 8 years ago
case class Effect(val id:Int) extends AnyVal
class Node {

  private var eff:Option[Effect] = None

  def effect_=(e:Option[Effect]) {
    if(e != null){
      eff = e
      println(e getOrElse "none provided")
    } else {
      eff = None
      println("none provided")      
    }
  }
  def effect_=(e:Effect) { effect_=(Option(e)) } //handles null as None
  def effect = eff
}

val n = new Node
n.effect = Effect(2)
n.effect = None
n.effect = null

Only for Optional / Nullable types

Original comment by dae...@gmail.com on 24 Sep 2013 at 4:11

GoogleCodeExporter commented 8 years ago
This issue was closed by revision 004359a3f7b5.

Original comment by jpsacha on 25 Sep 2013 at 2:16

GoogleCodeExporter commented 8 years ago
The NPE was caused by implicit conversion from sfx.Effect, with value `null`, 
to jfx.Effect.

Added fix for NPE and more general situation, without using Option:
https://code.google.com/p/scalafx/source/detail?r=004359a3f7b5

While `Option` is the way to avoid `null` in "pure" Scala code we deal here 
with wrapping of Java code that does not clearly indicate when `null` 
assignment is allowed. Adding use of `Option` in one place could lead to 
inconsistency in ScalaFX API. Note that `Option` can still be used in user 
code, if one chooses so.

Original comment by jpsacha on 25 Sep 2013 at 2:28