Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement NodeWalker utility for traversing configuration structures #103

Merged
merged 1 commit into from Jun 25, 2018

Conversation

lucko
Copy link
Contributor

@lucko lucko commented Jun 24, 2018

Provides a way to easily iterate over an entire configuration structure, or visit the child nodes in a specific sub section.

Use cases:

  • most obvious I can think of is "printing" a configuration to a user
  • searching for a specific value (less useful for configurations, but configurate can be used for reading and processing data too

I also intend to use this in SpongeCommon & GriefPrevention to exclude entries from being saved if they're already present & set to the same value in a "parent" config. Some sort of DFS post order traversal is needed to implement this behaviour, and I figured it made sense for that code to be in configurate itself, as opposed to being implemented in sub-projects.

Would appreciate any feedback :)

@lucko lucko force-pushed the feature/walk branch 2 times, most recently from 64b6f5e to c5b2393 Compare June 24, 2018 12:51
@liach
Copy link

liach commented Jun 24, 2018

can you make the node walker abstract class instead?

@lucko
Copy link
Contributor Author

lucko commented Jun 24, 2018

What would be the benefit?

@kashike
Copy link
Contributor

kashike commented Jun 24, 2018

I'd rather see a class like:

public abstract class NodeWalker {
    public static final NodeWalker BREADTH_FIRST = // ...
// ...

@lucko
Copy link
Contributor Author

lucko commented Jun 25, 2018

What would be the benefit?

@kashike
Copy link
Contributor

kashike commented Jun 25, 2018

Using an enum here restricts people from creating their own - can't extend an enum.

@lucko
Copy link
Contributor Author

lucko commented Jun 25, 2018

I don't foresee anyone doing this - but sure, point taken - will change.

Are you happy with the name NodeWalker? I'm not too sure on it

@kashike
Copy link
Contributor

kashike commented Jun 25, 2018

ConfigurationNodeWalker? ConfigurationNodeVisitor?

@lucko
Copy link
Contributor Author

lucko commented Jun 25, 2018

Hm yea, I think I prefer the first one

@lucko
Copy link
Contributor Author

lucko commented Jun 25, 2018

Okay, changed to ConfigurationNodeWalker and made the class abstract.

@kashike kashike self-assigned this Jun 25, 2018
@kashike kashike merged commit 59eee7a into master Jun 25, 2018
@lucko lucko deleted the feature/walk branch June 25, 2018 18:39
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.

None yet

3 participants