BUG: Listbox::findItemWithText skip start_item
Posted: Wed Jun 27, 2007 17:16
by zanir
Code: Select all
/*************************************************************************
Search the list for an item with the specified text
*************************************************************************/
ListboxItem* Listbox::findItemWithText(const String& text, const ListboxItem* start_item)
{
// if start_item is NULL begin search at begining, else start at item after start_item
size_t index = (!start_item) ? 0 : (getItemIndex(start_item) + 1);
while (index < d_listItems.size())
{
// return pointer to this item if it's text matches
if (d_listItems[index]->getText() == text)
{
return d_listItems[index];
}
// no matching text, advance to next item
else
{
index++;
}
}
// no items matched.
return 0;
}
I think is no logical to skip start_item in this function. For example:
pCombo->findItemWithText(pCombo->getText(),pCombo->getListboxItemFromIndex(0)); give different result as
pCombo->findItemWithText(pCombo->getText(), NULL); The solution is delete
+ 1 from first line in function.
Posted: Wed Jun 27, 2007 17:42
by Rackle
When you pass NULL you include the first element in the search.
Let's say that this first element matches your search; the function then returns that first element. However you now want to find the next entry that matches your text. You call this function again, passing the previously found item rather than null. Without the +1 the function would return that same item rather than searching the following elements.
Here's a concrete example, with the numbers representing the index position:
0. skip
1. text
2. text
3. skip
4. text
The function is called with:
ListboxItem* item = findItemWithText("text", 0)
this returns the item at index 1. Then you call the function with:
item = findItemWithText("text", item)
which now returns the item at index 2. Calling the function again with:
item = findItemWithText("text", item)
returns the last match, at index 4.
The d_listItems[index]->getText() == text test is a bit simplistic. I'd rather have a regular expression test such that a search for the text "*text*" would match "text 1", "text 2", "abracadabra text", etc.
Posted: Wed Jun 27, 2007 18:15
by zanir
I understand but maybe const ListboxItem* start_item should be named differently. For example start_skipped_item, start_and_skipped_item. Somebody can create code like:
Code: Select all
ListboxItem* item = findItemWithText("text", 0);
while(getItemIndex(item)+1<getItemCount() && item)
item = findItemWithText("text", getListboxItemFromIndex(getItemIndex(item)+1));
He/she will wonder why items are skipped and will find a bug.