Loading...

Code Review - Boolean's Delight

:heavy_exclamation_mark: This post is older than a year. Consider some information might not be accurate anymore. :heavy_exclamation_mark:

Did some code review and saw some fancy lines :-( It is about JSON handling with Jackson. Basically is about how to determine that a given JsonNode is not null and a container.

Snippet

/**
 * Check if {@link JsonNode} is not null and a container. (includes more JsonNodes)
 * @param jsonNode
 * @return true / false
 */
public static boolean isJsonNodeValidAndContainer(JsonNode jsonNode) {
	return jsonNode == null || !jsonNode.isContainerNode() ? false : true;
}

The intent is to avoid the necessary null check in each invocation.

Improvement 1: Since it is a condition, just return the result. The ? is superfluous.

/**
 * Check if {@link JsonNode} is not null and a container. (includes more JsonNodes)
 * @param jsonNode
 * @return true / false
 */
public static boolean isJsonNodeValidAndContainer(JsonNode jsonNode) {
	return (jsonNode == null || !jsonNode.isContainerNode());
}

What I personally don’t like, is the method name. It should be obvious and should have a single purpose. isValidContainer or isValidContainerNode (including the null check) sounds more reasonable to me.

The implementation of isContainerNode()

@Override
public final boolean isContainerNode() {
	final JsonNodeType type = getNodeType();
	return type == JsonNodeType.OBJECT || type == JsonNodeType.ARRAY;
}

See also the the current latest API doc.

Please remember the terms for blog comments.