Case classes and nulls /2

[cunei]

Hello Will,

Thank you for considering my pull request. The binary compatibility requirement is a good point; I am therefore submitting a modified version of the patch, which includes a simple optional configuration mechanism. The standard JacksMapper works exactly as it did before. However, it is now also possible to write:

import com.lambdaworks.jacks._
import JacksOption._

val m=JacksMapper.withOptions(CheckCaseClassNulls(true))
import m.{writeValueAsString,readValue}

Concerning its usefulness: in the common case in which a group of case classes are recursively deserialized from a hand-written JSON configuration file, for example, it is possible that the person writing the configuration file forgets one or more of the fields. In the absence of a library-supported option to automatically detect nulls, it is then upon the library client code to manually check that every individual field of every generated case class is not null. That, of course, adds a great deal of complexity to the user code (and makes it in turn error-prone).

I hope you will consider this pull request, or a variant, for inclusion (at least, it is crucial in my case). Thanks!

[diegovar]

I second this, this is Scala and usually nulls should be avoided like the plague. I find 2 behaviors of Jacks to be a bit disconcerting:

1. If I have

case class Person(name: String)

and I parse "{}" I get a Person(null). IMO this should fail instead of returning this Person object, whereas parsing "{name: null}" should not. Parsing should not fail if the field is an Option (map absent fields to None) or it has a default value (use the default one, that is correct today).

2. If I have

case class Person(name: String, carModel: Option[String], carPlate: String)

and I have an instance Person("John", None, null), I'd like it to be be able to serialize this as {"name": "John"}, ommitting the Nones and nulls, which I think are superflows and add only noise.

[diegovar]

Another argument in favor of this is that many people like us are probably migrating from Jerkson to Jacks, and thus this behavior would conflict with what Jerkson does with nulls and Nones. Jerkson did provide an option to explicitly serialize nulls, which was off by default.

[wg]

@diegovar thanks for your comments! I find it interesting that in (1) you want parsing to fail when a null property is omitted, and in (2) you want generation to omit properties that are null (carPlate). Aren't those mutually exclusive?

I've seen JSON that does both, some omits the property entirely when null, others include an explicit null, so I tend to think both cases should have the same behavior.

IMHO the correct way to treat optional JSON fields that may be omitted in Scala is to use default values, and not have a special case for nulls.

@cunei has a good point regarding checking for omitted fields as part of validation, but there's a lot more to validation than that and using existing code like scalaz validation seems more appropriate than trying to make jacks do validation.

As far as omitting nulls in JSON output, jacks supports Jackson's JsonInclude annotation at both class and field levels, so you can use Include.NON_NULL to skip nulls and Include.NON_EMPTY to skip Nones, empty collections, etc.

[diegovar]

On 1, I don't think they're mutually exclusive. It's how Jerkson worked, and how a library like Json4S works as well. Truth is, for the JSON blob you generate, writing a null is IMO equivalent to ommitting the property, only nastier. In most cases I believe you want to omit it, and in the cases where you want it you should be able to force it to appear always.

I encourage you to look at how Json4S works in these cases, I believe theirs is the most reasonable approach for most cases.

Diego