Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#16330 closed enhancement (fixed)

Use custom string comparison function with wxSortedArrayString

Reported by: catalin Owned by: VZ
Priority: normal Milestone:
Component: base Version: dev-latest
Keywords: Dictionary Lexicographical Cc:
Blocked By: Blocking:
Patch: yes

Attachments (1)

16330_changes.patch download (4.2 KB) - added by catalin 6 years ago.

Download all attachments as: .zip

Change History (7)

Changed 6 years ago by catalin

comment:1 follow-up: Changed 6 years ago by vadz

I've applied the rest of the patch but I'm confused by wxDictionaryStringSortAscending() implementation:

inline int wxCMPFUNC_CONV
wxDictionaryStringSortAscending(const wxString& s1, const wxString& s2)
{
    return s1.IsSameAs(s2, false) ? s1.Cmp(s2) : s1.CmpNoCase(s2);
}

As IsSameAs(false) is exactly the same as CmpNoCase() (but less readable), this means that we don't need to call CmpNoCase() the second time, we already know it's going to return 0 in this branch. Am I missing anything? Shouldn't the two branches of ?: be reversed?

comment:2 Changed 6 years ago by VZ

In 76750:

Change wxStringSort{As,De}cending() to use references, not pointers.

This is more convenient to use and makes more sense as the arguments are never
null.

See #16330.

comment:3 Changed 6 years ago by VZ

  • Owner set to VZ
  • Resolution set to fixed
  • Status changed from new to closed

In 76751:

Allow specifying custom comparator for wxSortedArrayString.

Add a possibility to order wxSortedArrayString in some order different from
the default alphabetical one.

Closes #16330.

comment:4 in reply to: ↑ 1 ; follow-up: Changed 6 years ago by catalin

Replying to vadz:

As IsSameAs(false) is exactly the same as CmpNoCase() (but less readable), this means that we don't need to call CmpNoCase() the second time, we already know it's going to return 0 in this branch. Am I missing anything? Shouldn't the two branches of ?: be reversed?

No, the branches should not be reversed. In a case sensitive comparison AAB would come before aaa so it needs case insensitive comparison (for identical string lengths); the other way around for BAA and a (different string lengths). But you're right with the double comparison; the function looks better as

inline int wxCMPFUNC_CONV
wxDictionaryStringSortAscending(const wxString& s1, const wxString& s2)
{
    int cmp = s1.CmpNoCase(s2);
    return cmp ? cmp : s1.Cmp(s2);
}
Last edited 6 years ago by catalin (previous) (diff)

comment:5 in reply to: ↑ 4 Changed 6 years ago by catalin

Replying to catalin:

In a case sensitive comparison AAB would come before aaa so it needs case insensitive comparison (for identical string lengths); the other way around for BAA and a (different string lengths).

Sorry, it's the same for different string lengths.
It only needs case sensitivity for aBa vs AbA vs ABa etc, so "equivalent" strings, if I can call then that.

comment:6 Changed 6 years ago by VZ

In 76753:

Add wxDictionaryStringSortAscending comparison function.

Add "dictionary sort" callbacks and document them and the already existing
wxStringSortAscending() and wxStringSortDescending().

See #16330.

Note: See TracTickets for help on using tickets.