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

Tree.hasChildren (with null safe refactor) #4

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

jpcaruana
Copy link

this method is very handy if you want to iterate over all the tree

@jkee
Copy link
Owner

jkee commented Feb 10, 2015

Thanks for pull request.
Why you can't just iterate through the tree using foreach?

@jpcaruana
Copy link
Author

Hi,

if you want to loop ever children, you first have to null check it: that's one unnecessary null check, one indent level more, less readability.

  • looping over en empty list removes all this clutter
  • having a hasChildren method is more explicit that checking nullity or emptiness, improving overall readability and hidding implementation details`

Or maybe I missed a way to do it already ?

@jkee
Copy link
Owner

jkee commented Feb 10, 2015

You can use treeIterator(). It'll handle nulls by itself.
I know that its usage is not so handy (no foreach) but it can be easily solved by returning Iterable (something like "asIterable" or just "flatten" method).

But if you want to loop children for only current node then it'll be better to have something like "getChildrenValues" which will return Collections.emptyList in null case.

I mean, there are two kind of operations - working with tree as a structure and working with tree as a container (accessing and editing elements). Unfortunately I wasn't be able to separate it in some clear way.

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.

2 participants