Reconsider deprecation of JsonNode.asText(defaultValue)

[aerisnju]

Describe your Issue

#4416 suggested deprecation of JsonNode.asText(defaultValue) because the default implementation of the method seemed to be useless according to its javadoc. But in my opinion, the javadoc didn't describe the behavior of the method accurately, which caused misunderstanding. It said:

... it will return defaultValue in cases where null value would be returned;

What did "null value would be returned" mean? Judging from its default implemenation, it seemed that when asText() returned a null value, the default value would be returned instead.

But this was not the truth. I believe you have noticed that some classes have overwritten the method, for example the NullNode and the MissingNode. But their implementation did not follow the javadoc, which was more confusing.

The problem is the description "null value would be returned". Actually, it does not mean asText() would return a null value, but it means the internal value of the node is null. If we take this understanding, we could easily understand the overwritten behavior of NullNode, MissingNode and POJONode. The NullNode and MissingNode definitely contain null values, and the POJONode may contain a null value (by the way, there's a similar but rare case in TextNode if it is constructed from null). All of the 3 classes overwrite the method to follow the rule described above.

Now we could understand the default implementation of the method. For a TextNode, there is a rare case that it may contain null values (asText() will return null in the case) and the implementation has covered the case. For other types of JsonNode, there should not be null values so the implementation actually forwards the method invocation to asText().

We can see that the motivation of asText(defaultValue) is clear but the implementation is a bit tricky. But this is not enough to persuade us to keep the method. We should find cases in which asText(defaultValue) is useful. Consider the case when we try to process a list of json objects in which some keys may be missing or null. We would like to display the data of the json objects in a table. If the corresponding column is missing or null, we will show a dash "-". With the help of asText(defaultValue), we just need to write:

String toShow = theObj.path("the corresponding key").asText("-"); // Show a dash - for values not available

Without the method:

JsonNode theNode = theObj.path("the corresponding key");
String toShow;
if (theNode.isNull() || theNode.isMissing()) {
  toShow = "-";
} else if (theNode.isPojo()) {
  Object value = ((POJONode) theNode).getPojo();
  if (value == null) {
    toShow = "-";
  } else {
    toShow = value.toString();
  }
} else {
  String value = theNode.asText();
  // Rare cases. Usually we can directly use asText() but the code below makes our program more robust.
  if (value == null) {
    toShow = "-";
  } else {
    toShow = value;
  }
}

We can see that asText(defaultValue) provides a convenient and natural way of string conversion with defaults for missing or null values. At the same time, it provides "non-null" guarantee for getting text values by a non-null default value, and "null-if-not-available" behavior by passing a default null value.

Finally, I suggest keeping the method and clarifying its javadoc description. or deprecating the current name but renaming it to describe the behavior more clearly.

[JooHyukKim]

Finally, I suggest keeping the method and clarifying its javadoc description. or deprecating the current name but renaming it to describe the behavior more clearly.

JavaDoc improvement PR is always welcome.

[cowtowncoder]

+1 for Javadoc improvement (PR could go in 2.17 branch)

I am also open for un-deprecation (for 2.x), based on improved description -- I agree with the logic.
This would need to go in 2.18 as it is of API change.

I am not yet 100% sure if this should mean re-introduction of asText(defaultValue) for 3.0 (master)... I guess it should, if method not marked as Deprecated in 2.x.

[JooHyukKim]

PTAL at #4476 @aerisnju ? May be clearer than before?

[JooHyukKim]

Status Update : Pending

Reconsideration of un-deprecation is to be discusssed with the issue author

[cowtowncoder]

I think it'd make sense to undeprecate this in 2.17.1, due to valid points wrt usefulness -- the issue was more with originally misleading Javadoc than the functionality.

[cowtowncoder]

PR to revert: #4419

[aerisnju]

I think it should be OK to revert the deprecation. Thank you.

[cowtowncoder]

Undeprecated in 2.17 branch for upcoming 2.17.1 release.