Handling NPE

[Chung Onn Cheong]

I found scalatags does not handle NPE condition well. It is very hard chasing after the offensive tag when NPE occurs especially when the markup is large. It would nice if a div(null) can be rendered as <div>null</div> instead of throwing an NPE. I managed to figure it out that I need only to apply a one line fix in the case class StringFrag#writeTo. Should I do a PR?

case class StringFrag(v: String) extends text.Frag{

    def render = {

      val strb = new StringBuilder()

      writeTo(strb)

      strb.toString()

    }

    def writeTo(strb: StringBuilder) = Escaping.escape(if (v == null) "null" else v, strb)

  }

[Haoyi Li]

Is that really all that's necessary? It seems to me that you'd need to put the same thing into every other XXXFrag out there =P

--
You received this message because you are subscribed to the Google Groups "Scalatags" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scalatags+...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[Chung Onn Cheong]

Is that really all that's necessary? It seems to me that you'd need to put the same thing into every other XXXFrag out there =P

I certainly hope so and I too wonder the same thing . You would be the best person to comment :) 

FYI, I failed to make Scalatags project as a dependency and I am not able to debug the inner workings of Scalatags using both Eclipse and Intellij as both IDE complained of missing source code, I trust this is due to the Scala macro. My only option is through trial and error and relying of the stacktrace. I believe StringFrag would be the most common 'null' scenario, and the other modifier like Tag or AttrPair  being null would be rare.

Just curious, what tool do you use for your development? 

[Chung Onn Cheong]

I did one more change below is the diff output, with the changes here's what I get 

scala> div(null)

res0: scalatags.Text.TypedTag[String] = <div></div>

diff --git a/scalatags/shared/src/main/scala/scalatags/Text.scala b/scalatags/shared/src/main/scala/scalatags/Text.scala

index 583aa0f..112ef1b 100644

--- a/scalatags/shared/src/main/scala/scalatags/Text.scala

+++ b/scalatags/shared/src/main/scala/scalatags/Text.scala

@@ -97,7 +97,7 @@ object Text

       writeTo(strb)

       strb.toString()

     }

-    def writeTo(strb: StringBuilder) = Escaping.escape(v, strb)

+    def writeTo(strb: StringBuilder) = Escaping.escape(if (v == null) "null" else v, strb)

   }

   object StringFrag extends Companion[StringFrag]

   object RawFrag extends Companion[RawFrag]

diff --git a/scalatags/shared/src/main/scala/scalatags/generic/Core.scala b/scalatags/shared/src/main/scala/scalatags/generic/Core.scala

index d20519e..5e396c4 100644

--- a/scalatags/shared/src/main/scala/scalatags/generic/Core.scala

+++ b/scalatags/shared/src/main/scala/scalatags/generic/Core.scala

@@ -70,7 +70,8 @@ trait TypedTag[Builder, +Output <: FragT, +FragT] extends Frag[Builder, FragT]{

       val frag = arr(j)

       var i = 0

       while(i < frag.length){

-        frag(i).applyTo(b)

+        val f = frag(i)

+        if(f != null) f.applyTo(b)

         i += 1

       }

     }

[Ben Hutchison]

To me, silently converting nulls to strings isn't *nice* at all. Very rails.

I'd prefer to know asap if I have nulls floating around.

Ben

[Chung Onn Cheong]

I only realized this morning that I replied to Ben instead to the mailing list. Below is my response to Ben

It would not be nice when a page rendered that very unforgivingly tell user that an NPE occured with a stack dump compared to display of a "null" string which the developer can quickly zoom in to rectify it. :)

It would be painful if we have to check for null value before using scala tag. Scalatags very nicely handle None and Unit condition, shouldn't it handle the infamous null?

With my 3 weeks experience in scalatags I'm only able to get StringFrag show up as a "null" string and others just an empty String.

[Ben Hutchison]

I offer this as explanation, not debate, because its a pretty dry argument:

Collectively, progammers have learned through experience that in the end, we are far better off controlling nulls at the source than as symptoms.

You speak of nulls as an unavoidable fact of life. They shouldn't be. Dont tolerate them. If something is optional, use Option. Otherwise, never allow a value to be null. If its coming from outside, validate it at your boundaries.

-Ben

[Chung Onn Cheong]

You missed my point about a library allowing graceful degradation, while harp on good programming practice. Below shows 3 scenarios which I think a nice library should be handling the 3rd scenario gracefully. Anyway, it's Haoyi's prerogative to decide whether to accept my PR or not. I am happy with whatever decision he makes. This is a free world, we need to able to respect and accommodates alternate views.

One last thing, I never view your comment as a debate. And have made remark like "very rails" is very opinionated :).

scala> import scalatags.Text.all._

import scalatags.Text.all._

scala> div(if(nullable != null) nullable else "")

res1: scalatags.Text.TypedTag[String] = <div></div>

scala> div(Option(nullable))

res2: scalatags.Text.TypedTag[String] = <div></div>

scala> div(nullable)

res3: scalatags.Text.TypedTag[String] = <div>null</div>