eXsio / nestedj

NestedJ - a NestedSet implementation for Java
MIT License
95 stars 24 forks source link

PrevSibling/NextSibling #2

Closed ghost closed 5 years ago

ghost commented 5 years ago

Hey are you by any chance interested in a pull request that adds methods and related unit tests for retrieving next and previous sibling nodes?

https://github.com/rollenwiese/nestedj/commit/ad56143678261ed7f0d4bf0b8984a657738fa169

The _getsiblings branch in my repo should merge without issue.

eXsio commented 5 years ago

Hello

Thanks for taking the interest in nestedj :)

Your changes look interesting and I would welcome the PR that contains them. There's just one thing I was wandering - do we really need the following condition?:

if (node.getTreeLevel() > 0)

What if I'd like to retrieve the next root element? If I remember correctly NestedJ doesn't have a limitation on a number of root elements - there is no single 0-leveled root node enforced.

Could you check if your logic can work also on 0-leveled nodes and add tests for it?

Cheers :) eXsio

śr., 4 wrz 2019, 19:32 użytkownik gregory gincley notifications@github.com napisał:

Hey are you by any chance interested in a pull request that adds methods and related unit tests for retrieving next and previous sibling nodes?

rollenwiese@4dc4d9c https://github.com/rollenwiese/nestedj/commit/4dc4d9cea06941fb3b959ca399e461bcac1759bc

There's a couple of other commits in the same branch you would have to ignore.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/eXsio/nestedj/issues/2?email_source=notifications&email_token=AAO2E4N5SUTY7257WZ7CEVDQH7WLPA5CNFSM4ITUTBYKYY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4HJKBNOA, or mute the thread https://github.com/notifications/unsubscribe-auth/AAO2E4JU7LR3H4YEEB6TFPLQH7WLPANCNFSM4ITUTBYA .

ghost commented 5 years ago

Hi exSio, Thanks I do appreciate this library.

I updated the methods in the NodeRetriever, and created some additional tests for the Root Node scenario. Please let me know what you think.

Updated commit: https://github.com/rollenwiese/nestedj/commit/5a2acd9415e4953c3ab998bc7f60815a38f84564

Thanks, -Greg

eXsio commented 5 years ago

Hello

Looks good, feel free to create PR.

Thanks :) eXsio

czw., 5 wrz 2019, 19:00 użytkownik gregory gincley notifications@github.com napisał:

Hi exSio, Thanks I do appreciate this library.

I updated the methods in the NodeRetriever, and created some additional tests for the Root Node scenario. Please let me know what you think.

Updated commit: rollenwiese@5a2acd9 https://github.com/rollenwiese/nestedj/commit/5a2acd9415e4953c3ab998bc7f60815a38f84564

Thanks, -Greg

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/eXsio/nestedj/issues/2?email_source=notifications&email_token=AAO2E4JMNJZUAJX2LOFJDUDQIE3KVA5CNFSM4ITUTBYKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD573B5Q#issuecomment-528462070, or mute the thread https://github.com/notifications/unsubscribe-auth/AAO2E4JWVGYEDYL5NRT4ZLDQIE3KVANCNFSM4ITUTBYA .

ghost commented 5 years ago

Will do, and thanks again for creating this library!

eXsio commented 5 years ago

PR merged, closing. Thx for the improvement.

eXsio commented 5 years ago

Travis Build confirmed stable