3
\$\begingroup\$

This function removes a JSON element from a JSON file. In may case, the ID element. I am able to get the desired result. But the code looks very verbose; can it be written in a better way without hindering the performance?

The path for removal is provided from the root node: /glossary/GlossDiv/GlossList/GlossEntry/ID

{
 "glossary": {
 "title": "example glossary",
 "GlossDiv": {
 "title": "S",
 "GlossList": {
 "GlossEntry": {
 "ID": "SGML",
 "SortAs": "SGML",
 "GlossTerm": "Standard Generalized Markup Language",
 "Acronym": "SGML",
 "Abbrev": "ISO 8879:1986",
 "GlossDef": {
 "para": "A meta-markup language, used to create markup languages such as DocBook.",
 "GlossSeeAlso": ["GML", "XML"]
 },
 "GlossSee": "markup"
 }
 }
 }
 }
}

The code to remove the element follows. As one can observe, it has two for loops and we are traversing to the parent of the element which needs to be removed and then removing the element. Is there a better way of writing the code?

public static JsonNode removeProperty(JsonNode node, List<String> removedField){
 JsonNode modifiedNode = node;
 for (String nodeToBeRemoved: removedField){
 String[] array = nodeToBeRemoved.split("/");
 for (int i =1;i<array.length-1;i++){
 String name=array[i];
 modifiedNode = modifiedNode.get(name);
 }
 ((ObjectNode)modifiedNode).remove(array[array.length-1]);
 }
 return node;
}
Toby Speight
87.6k14 gold badges104 silver badges325 bronze badges
asked Jan 6, 2022 at 20:46
\$\endgroup\$

1 Answer 1

3
\$\begingroup\$

One thing I would suggest is to split this into a few functions with descriptive names and smaller scopes. First, since we can remove several fields with this function, I think it would be more clearly named removeProperties. (Also, is there a reason that the function name uses "property" and the parameter name uses "field"? It seems to me you could pick one or the other.)

If you split this into single responsibility functions, you get:

public static JsonNode removeProperties(JsonNode node, List<String> removedField){
 JsonNode modifiedNode = node;
 for (String nodeToBeRemoved: removedField){
 modifiedNode = removeProperty(modifiedNode, nodeToBeRemoved);
 }
 return node;
}

A second observation is that it looks like ObjectNode.remove mutates the node. If that's correct, then why bother returning the modified node? Since it's changed in place, you could make this a void function. Or, if the intent is to avoid mutating the provided node, then you'd need to clone it before starting to operate on it. Even if you clone it, remove still changes the node in place, so there's no need for the inner function to return modifiedNode.

I suggest you include JavaDocs specifying whether this function will change the input node or not.


Thirdly, I suggest some renaming here:

String[] array = nodeToBeRemoved.split("/");

To me, this would me more clear as follows, since it makes sense to split a path but I don't know what it means to split a node. We already have a node in context, and its type is JsonNode. If we also call this string a node, then we have two things called "node" which have different types.

String[] pathParts = pathToBeRemoved.split("/");

On the topic of names, I think it would be helpful to extract a variable for the last element:

String keyToRemove = pathParts[pathParts.length - 1];

That way the remove line becomes more clear:

((ObjectNode) nodeToModify).remove(keyToRemove);
answered Jan 6, 2022 at 23:20
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.