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

Add EasyBind.concat(List<ObservableList<T>>) #8

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

Conversation

maul-esel
Copy link

Creates a new list that combines the values of the given lists. Unlike FXCollections.concat(), updates to the source lists propagate to the combined list.

I needed this for a project of mine, so I hacked it together. Since it might be useful to others as well, I thought I'd submit it here. If there are any changes you'd like me to make, let me know.

Creates a new list that combines the values of the given
lists. Unlike FXCollections.concat(), updates to the source
lists propagate to the combined list.
@TomasMikula
Copy link
Owner

Thanks for the PR!

OMG, I didn't expect FXCollections.concat() to be so silly!

I can see how your implementation would fail on

concat(list, list);

Some tests would be nice.

@maul-esel
Copy link
Author

I added some tests, hope they're ok. Otherwise let me know.

I also added a short test for concat(list, list); and so far it seems to work. What part exactly did you expect to fail? (maybe I didn't cover that)

@TomasMikula
Copy link
Owner

You did not test that the published change events are correct.

@N247S
Copy link

N247S commented Apr 9, 2018

Just as an idea, wouldn't it be better (for boilersplate's sake) to switch out the sourceLists field in ConcatList with a plain array? As it's immutable, and operations performed on the List are easily done as array operations (This would also means the concat method in EasyBind can be merged into a single varargs method).

Second (for performance sake), would it be an idea to cage the overal size? As it can be detected when the underlaying lists are changing, it would easy to refresh/recalculate the overal size (or at least set a flag that the current size is invalid), whichever way is prefered.

tobiasdiez referenced this pull request in tobiasdiez/EasyBind May 5, 2020
* Add EasyBind.concat(List<ObservableList<T>>)

Creates a new list that combines the values of the given
lists. Unlike FXCollections.concat(), updates to the source
lists propagate to the combined list.

* Fix compiler errors

* add some tests for concatenating lists

* Fix bugs in the ConcatList implementation

* Duplicated List values will break the implementation's change events
* Added better tests that test different combinations.

This commit closes comments in #8.

* Add .idea to .gitignore, clean up test case

* Added support for an "ObservableList" input

* Adds an ObservableList input
* Tracks changes to the List, firing changes if the 2D list changes,
  this eases up the work when bigger changes happen
* Change `concat` -> `flattenList`, it's a flatten operation after all

* Rename flatten to concat

Co-authored-by: maul.esel <maul.esel@go4more.de>
Co-authored-by: Kevin Brightwell <kevin.brightwell2@gmail.com>
@tobiasdiez
Copy link

Since this project sadly doesn't seem to be active any more #16, I've created a fork and continued development there: https://github.com/tobiasdiez/EasyBind.

I liked your changes in this PR, and thus included them in the forked repo. Hope this is OK for you.

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

4 participants