Skip to content

Conversation

@eslavich
Copy link
Contributor

@eslavich eslavich commented Feb 8, 2026

This adds convenience methods for fetching nodes deep in the tree. For example, until now it has been necessary to chain method calls to get to a node at path root.child.grandchild:

node.get("root").get("child").get("grandchild")

This PR adds vararg overloads of the get methods so that this can be accomplished in a single method call:

node.get("root", "child", "grandchild")

Since varargs can't be followed by other arguments, I'm also deprecating the old getList and getMap methods and introducing new methods that place the list and map types at the beginning of the argument list.

Thanks Trey for the idea!

@github-actions
Copy link

github-actions bot commented Feb 8, 2026

Overall Project 57.82% -0.49% 🍏
Files changed 88.71% 🍏

Module Coverage
asdf-core 57.88% -0.5% 🍏
Files
Module File Coverage
asdf-core NumberAsdfNode.java 97.6% -1.54% 🍏
ReferenceFileUtils.java 85.85% -0.98% 🍏
AsdfNodeBase.java 63.88% -4.4% 🍏

Copy link

@robyww robyww left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good. I think it will make the library easier to use. thanks!.


final AsdfNode currentKey = constructKeyNode(keys[0]);

return get(currentKey).get(Arrays.copyOfRange(keys, 1, keys.length));
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can see a reason to copy the array for this operation. If upoi were going to modify it or it was threaded it might make sense but it does not appear necessary here.

I see this in other places as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I need to pass the array without its first element, is there a better way to do that? I could add an additional get(...) overload that accepts the array start index.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh, I missed that. That makes sense.

Copy link

@alphasentaurii alphasentaurii left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good - appreciate the added test case for this as well. Thanks Ed!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants