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 listbox::items() method to get all items index_pairs #613

Open
wants to merge 1 commit into
base: develop-1.8
Choose a base branch
from

Conversation

riquems
Copy link

@riquems riquems commented Mar 12, 2021

Hi, I had problems into getting all items from a listbox, so I came with this idea of contributing to the project. It's a very small change but I think it's very useful :).

If the functionality already exists let me know, I found only one way to reproduce what I wanted:
Select the category items and then get the selected items (which wasn't very intuitive at first and it works only for individual categories), like:

lb_listbox1->at(0).select(true);
listbox::index_pairs items = lb_listbox1->selected();

Now I can do this:

listbox::index_pairs items = lb_listbox1->items();

Anyway, It worked in my tests, feel free to edit/implement in your way, I just wanted to see this feature in the library. Thanks for your awesome job.

@cnjinhao
Copy link
Owner

I just wonder why we need this method. IMO, it looks like we get all indexes of an array.

@riquems
Copy link
Author

riquems commented May 21, 2021

I just wonder why we need this method. IMO, it looks like we get all indexes of an array.

I mean, it's self explanatory actually: get all items from the listbox (I know it's only the indexes, but the function selected() does the same). If there's another cleaner way to do this... I don't know.

If you can get the selected items from the listbox, why can't you get all of them? :)

@qPCR4vir
Copy link
Collaborator

qPCR4vir commented Mar 29, 2022

Hi, @riquems ...
I will try to come back... not sure yet.
How are you going to use the returned list of all indexes?
It is not he same to try to get "all the items" than to try to get the items the user selected and becouse you need to do something only with that (there is no other way to get that). "All the items" are always there, there is no particular selection from the user.
The user make a selection of items and then click something and expect the program uses that selection: you "stop", get the indexes, and do that.
If you want to use all the items you don't need to ask: the list has all of then. Maybe, there could be a better way to visit each of then. But to keep a list of all indexes don't seems to be very good idea. If you have an array you don't need a list of the indexes to work with the items in the array. If somebody have a selection of items in the array, well, you will need to get a list of the indexes of the seleted items.
To visit each item you could:

for (int c=0; c<listb.size_categ(); c++)
  for (int i=0; i<listb.size_item(c); i++)
    listb[c][i]; // the items

I agree, it could be better (to be implemented):

for (auto &cat : listb)
  for (auto &i : cat)
    i;  //  the items

@riquems
Copy link
Author

riquems commented Jan 9, 2023

Hi @qPCR4vir, thanks for coming back! This is my use case:

image

I have two listboxes, and I can move items from one to the other. The simple arrow head (> and <) moves only selected items, and the dual arrow heads (>> and <<) moves all the items. To do this, I figured it out by using index_pairs and basically moving the items from one list to the other (welp, thats not so descriptive, here is my demo code: https://pastebin.com/ckG8jbDD)

Without the method I implemented, the code that moves all items looks like:

lb1.at(0).select(true);
auto items = lb1.selected();

move_items(items, lb1, lb2);

This can be found in line 29 of the pastebin code.

With the method I implemented, you can do this:

auto items = lb1.items();

move_items(items, lb1, lb2);

Which (I believe) is more readable.

I didn't know we could iterate the listbox items like you did with the for each thing. That's cool, but imperative. A method to return these in a way we can populate another listbox with the same items, is still better.

Welp, maybe my code isn't ideal either, that's the way I did it though. I'm open for suggestions for improving my code (if I went the hard path), or suggest another possible better way of doing it. I just went conservative and followed what has been already done in the .selected() method.

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