Lexicographical string sorting [for wxSortedArrayString too]

33 views
Skip to first unread message

Catalin

unread,
Jun 3, 2014, 3:19:03 AM6/3/14
to wx-...@googlegroups.com
Hello,

I've found several definitions for this kind of sorting, and IIUC they describe case sensitive sorting. So basically the following strings are in the "correct" order:
Aaa
aaa
a
Aba
aba
b

But this is not the correct order in any dictionary that I could find online. The correct order in a dictionary is:
a
Aaa
aaa
Aba
aba
b

A quick conclusion is that the comparison function should be something like this:
int dictionaryCompare(const wxString& s1, const wxString& s2)
{
return s1.length() == s2.length() ? s1.Cmp(s2) : s1.CmpNoCase(s2);
}


wxSortedArrayString uses std::sort which by default is a case sensitive comparison, and AFAICS it cannot be changed.
There is no unit test for this case since all strings used in the present tests have different characters as first letter.

Should wxSortedArrayString be changed to use the lexicographical / dictionary sorting or should another wxDictionarySortedArrayString class be added ? This would be useful in a few places, including the wxListBox and wxComboBox sorting under wxUniv. 


Regards,
C


p.s. Would it make sense to change wxSortedArrayString::Insert(const wxString& str, size_t nIndex, size_t copies) to call Add(str, copies) and ignore nIndex?

Eric Jensen

unread,
Jun 3, 2014, 3:38:03 AM6/3/14
to 'Catalin' via wx-dev
Hello 'Catalin',

Tuesday, June 3, 2014, 9:19:01 AM, you wrote:

Cvwd> wxSortedArrayString uses std::sort which by default is a
Cvwd> case sensitive comparison, and AFAICS it cannot be changed.

Cvwd> There is no unit test for this case since all strings
Cvwd> used in the present tests have different characters as first
Cvwd> letter.

Cvwd> Should wxSortedArrayString be changed to use
Cvwd> the lexicographical / dictionary sorting or should
Cvwd> another wxDictionarySortedArrayString class be added ? This
Cvwd> would be useful in a few places, including the wxListBox and
Cvwd> wxComboBox sorting under wxUniv. 

Just a warning: Once you start implementing your own sort-function,
things become non-trivial when you have to sort non-ascii chars.

Eric


Catalin

unread,
Jun 3, 2014, 4:02:23 AM6/3/14
to wx-...@googlegroups.com
Hello Eric,



On Tuesday, 3 June 2014, 10:38, Eric Jensen wrote:
Just a warning: Once you start implementing your own sort-function,
things become non-trivial when you have to sort non-ascii chars.
Indeed. However I tried to avoid that by using the already existing wxString::Cmp() and wxString::CmpNoCase().
Maybe there are even better ways, but the main question is what is the expected behavior.

Regards,
C

Eric Jensen

unread,
Jun 3, 2014, 5:55:28 AM6/3/14
to 'Catalin' via wx-dev
Hello 'Catalin',

Tuesday, June 3, 2014, 10:02:22 AM, you wrote:
Cvwd> On Tuesday, 3 June 2014, 10:38, Eric Jensen wrote:
Cvwd> Just a warning: Once you start implementing your own sort-function,
>>things become non-trivial when you have to sort non-ascii chars.
>>Indeed. However I tried to avoid that by using the already
>>existing wxString::Cmp() and wxString::CmpNoCase().
Cvwd> Maybe there are even better ways, but the main question
Cvwd> is what is the expected behavior.

What i meant is that if you start implementing your own sort function,
you're opening a can of worms. You should utilize the OS functions if
possible.

E.g. here's a comparion between standard wxArrayString and wxArrayString using Windows' string compare function (German locale):

11:47:23: standard sort
11:47:23: A
11:47:23: AAA
11:47:23: AAa
11:47:23: Aaa
11:47:23: Ba
11:47:23: Bc
11:47:23: Bd
11:47:23: Bć
11:47:23: a
11:47:23: aa
11:47:23: aaa

11:47:23: Windows sort
11:47:23: a
11:47:23: A
11:47:23: aa
11:47:23: aaa
11:47:23: Aaa
11:47:23: AAa
11:47:23: AAA
11:47:23: Ba
11:47:23: Bc
11:47:23: Bć
11:47:23: Bd

Basically the standard function just does a memcmp.

Eric



Hans Mackowiak

unread,
Jun 3, 2014, 5:58:20 AM6/3/14
to wx-...@googlegroups.com
Am Dienstag, den 03.06.2014, 00:19 -0700 schrieb 'Catalin' via wx-dev:
> A quick conclusion is that the comparison function should be something
> like this:
> int dictionaryCompare(const wxString& s1, const wxString& s2)
> {
> return s1.length() == s2.length() ? s1.Cmp(s2) : s1.CmpNoCase(s2);
> }
>
>
lexicon is a bit more complicated ... specially in other contries that
has umlauts like "äöü" because "ä" is sorted as if it would be "ae"
same for "ß" is treat as "ss"

Eric Jensen can tell you more about that because he is german too. (and
swicherland and some other contries has different lexi for the same
umlaut so its local depending)

Thanks,

Hanmac


Igor Korot

unread,
Jun 3, 2014, 6:07:00 AM6/3/14
to wx-...@googlegroups.com
Hi,

On Tue, Jun 3, 2014 at 2:58 AM, Hans Mackowiak <han...@gmx.de> wrote:
> Am Dienstag, den 03.06.2014, 00:19 -0700 schrieb 'Catalin' via wx-dev:
>> A quick conclusion is that the comparison function should be something
>> like this:
>> int dictionaryCompare(const wxString& s1, const wxString& s2)
>> {
>> return s1.length() == s2.length() ? s1.Cmp(s2) : s1.CmpNoCase(s2);
>> }
>>
>>
> lexicon is a bit more complicated ... specially in other contries that
> has umlauts like "äöü" because "ä" is sorted as if it would be "ae"
> same for "ß" is treat as "ss"

And then there are different Chinese/Japanese/Tai languages....

Thank you.

>
> Eric Jensen can tell you more about that because he is german too. (and
> swicherland and some other contries has different lexi for the same
> umlaut so its local depending)
>
> Thanks,
>
> Hanmac
>
>
> --
> To unsubscribe, send email to wx-dev+un...@googlegroups.com
> or visit http://groups.google.com/group/wx-dev
>

Vadim Zeitlin

unread,
Jun 3, 2014, 7:21:25 AM6/3/14
to wx-...@googlegroups.com
On Tue, 3 Jun 2014 00:19:01 -0700 (PDT) 'Catalin' via wx-dev wrote:

Cvwd> A quick conclusion is that the comparison function should be
Cvwd> something like this:
Cvwd> int dictionaryCompare(const wxString& s1, const wxString& s2)
Cvwd> {
Cvwd> return s1.length() == s2.length() ? s1.Cmp(s2) : s1.CmpNoCase(s2);
Cvwd> }

This would be one possibility.

Cvwd> wxSortedArrayString uses std::sort which by default is a case
Cvwd> sensitive comparison, and AFAICS it cannot be changed.

You can use a custom comparator with std::sort() doing case-insensitive
comparison.

Cvwd> There is no unit test for this case since all strings used in the
Cvwd> present tests have different characters as first letter.

The existing unit test should be extended to check for it then. And we
probably need to behave the other platforms in the same way as wxMSW, as it
can't really be customized.

Cvwd> Should wxSortedArrayString be changed to use the lexicographical /
Cvwd> dictionary sorting or should
Cvwd> another wxDictionarySortedArrayString class be added ? This would be
Cvwd> useful in a few places, including the wxListBox and wxComboBox
Cvwd> sorting under wxUniv. 

We shouldn't change wxSortedArrayString behaviour but adding a new class
just for this seems like an overkill. I think the least intrusive solution
would be to allow specifying the comparator function in wxSortedArrayString
ctor.

Cvwd> p.s. Would it make sense to change wxSortedArrayString::Insert(const
Cvwd> wxString& str, size_t nIndex, size_t copies) to call Add(str, copies)
Cvwd> and ignore nIndex?

According to the comment before WX_DEFINE_SORTED_TYPEARRAY definition in
wx/dynarray.h, Insert() shouldn't be defined at all for this class... If it
is, there must be a bug somewhere but I'm not brave enough to go looking
for it right now.

I wonder if we can rewrite these macros using templates one day...

Regards,
VZ

petah

unread,
Jun 3, 2014, 7:31:22 AM6/3/14
to wxDev
On Tue, 03 Jun 2014 11:58:24 +0200
Hans Mackowiak <hanmac-Mm...@public.gmane.org> wrote:
> lexicon is a bit more complicated ... specially in other contries that
> has umlauts like "äöü" because "ä" is sorted as if it would be "ae"
> same for "ß" is treat as "ss"
>
> Eric Jensen can tell you more about that because he is german too. (and
> swicherland

Now, what mysterious country could that be? :)

> and some other contries has different lexi for the same
> umlaut so its local depending)

Switzerland doesn't have a distinct lexicon for Schwyzerdütsch because it's not an official written language (though Romanche is) and isn't taught in school. AFAIK written Swiss-German is Hochdeutsch.

Stroustrup's C++11 book dedicates a LOT of space to "culturally-sensitive character handling" - not just compares for sorting but other nonsense like de/serializing numbers with a comma decimal separator for UK and a dot for US - but I doubt such a complex issue can be transparently handled at low level. You can't possibly compare strings from different alphabets. F.ex. non-Latin names are usually treated as phonetic approximations.

IMHO memcmp() is just fine for default behaviour, anything fancier would be handled at the application level.

cheers,

-- p

Catalin

unread,
Jun 3, 2014, 8:28:37 AM6/3/14
to wx-...@googlegroups.com
On Tuesday, 3 June 2014, 14:21, Vadim Zeitlin <va...@wxwidgets.org> wrote:
On Tue, 3 Jun 2014 00:19:01 -0700 (PDT) 'Catalin' via wx-dev wrote:

Cvwd> wxSortedArrayString uses std::sort which by default is a case
Cvwd> sensitive comparison, and AFAICS it cannot be changed.

You can use a custom comparator with std::sort() doing case-insensitive
comparison.
I don't think there is a way currently to do that using wxSortedArrayString. Again, I'm asking all these regarding the use of wxSortedArrayString because of the advantage of automatically sorting its contents when new elements are added.

Is 
int dictionaryCompare(const wxString& s1, const wxString& s2)
{

     return s1.length() == s2.length() ? s1.Cmp(s2) : s1.CmpNoCase(s2);
}
good enough for wxUniv which has no "native" sort function?

I think the least intrusive solution would be to allow specifying the comparator function in wxSortedArrayString ctor.
Yes, that should be good.

Cvwd> p.s. Would it make sense to change wxSortedArrayString::Insert(const
Cvwd> wxString& str, size_t nIndex, size_t copies) to call Add(str, copies)
Cvwd> and ignore nIndex?

According to the comment before WX_DEFINE_SORTED_TYPEARRAY definition in
wx/dynarray.h, Insert() shouldn't be defined at all for this class... If it
is, there must be a bug somewhere but I'm not brave enough to go looking
for it right now.
wxSortedArrayString derives from wxArrayString so it does have it; the docs say that wxSortedArrayString::Insert() "should not be used".
Anyway, I was just curious about this.

I wonder if we can rewrite these macros using templates one day...
Would that be needed, now with wxVector<> and better compilers that can digest std::vector? Or just to have a sorted version of vectors?

Regards,
C

Vadim Zeitlin

unread,
Jun 3, 2014, 8:46:26 AM6/3/14
to wx-...@googlegroups.com
On Tue, 3 Jun 2014 05:28:35 -0700 (PDT) 'Catalin' via wx-dev wrote:

Cvwd> I don't think there is a way currently to do that using wxSortedArrayString. Again, I'm asking all these regarding the use of wxSortedArrayString because of the advantage of automatically sorting its contents when new elements are added.
Cvwd> Is int dictionaryCompare(const wxString& s1, const wxString& s2)
Cvwd> {
Cvwd> return s1.length() == s2.length() ? s1.Cmp(s2) : s1.CmpNoCase(s2);
Cvwd> }
Cvwd> good enough for wxUniv which has no "native" sort function?

Again, I think the important thing is to make it behave like wxMSW. If
this is what wxMSW does, then it should be fine. If not, it really depends
on what exactly does wxMSW do, if it's something remotely reasonable we
should try to emulate it. If it's something that really doesn't make sense,
we should do the above and document the difference.

Cvwd> wxSortedArrayString derives from wxArrayString so it does have it;
Cvwd> the docs say that wxSortedArrayString::Insert() "should not be
Cvwd> used".Anyway, I was just curious about this.

In this case it should indeed be changed as you propose...

Cvwd> I wonder if we can rewrite these macros using templates one day...
[BTW, the text part of your message doesn't make any distinction between
what I wrote and what you replied, which results in wrong quoting in
replies, without speaking of a difficulty with reading them as text. I
wonder if you could configure Yahoo to send your emails to wx-dev as text
only?]
Cvwd> Would that be needed, now with wxVector<> and better compilers that
Cvwd> can digest std::vector? Or just to have a sorted version of vectors?

Yes, having an always sorted container is often helpful. And there is also
the issue of compatibility, of course: it's much simpler to change
wxArrayString implementation to be more readable and debuggable than to
change all the places where it is used...

Regards,
VZ

Catalin

unread,
Jun 3, 2014, 5:01:09 PM6/3/14
to wx-...@googlegroups.com
On Tuesday, 3 June 2014, 13:32, Eric Jensen wrote:


E.g. here's a comparion between standard wxArrayString and wxArrayString using Windows' string compare function (German locale):
Do you have link with the reference of Windows' string compare function?

Thanks,
C

Vadim Zeitlin

unread,
Jun 3, 2014, 5:05:38 PM6/3/14
to wx-...@googlegroups.com
On Tue, 3 Jun 2014 14:01:07 -0700 (PDT) 'Catalin' via wx-dev wrote:

Cvwd> Do you have link with the reference of Windows' string compare function?

FWIW I don't think you need it, even assuming it exists and I don't think
that all locale rules are documented anywhere. It can be fine that the
strings appear in different order on different platforms provided they
still follow the platform convention. And I am almost certain (I just can't
find the example that I saw of this happening right now) that in some
locales Windows and OS X collate strings differently. So we'll just have to
live with this.

There is also the general brokenness of Unix layer of OS X, which doesn't
support locales as well as Core Foundation (or at all in some cases). So
there are definitely things to improve here, e.g. we ought to have a
UIStringCompare function, or rather UIStringComparator object, which could
point to different instances in console applications and GUI ones. But this
is getting way too involved.

If we can just ensure that "a" compares the same with "AA" under all
platforms, this would be good enough for me.

Regards,
VZ

Eric Jensen

unread,
Jun 3, 2014, 5:05:56 PM6/3/14
to 'Catalin' via wx-dev
Hello 'Catalin',

Tuesday, June 3, 2014, 11:01:07 PM, you wrote:

Cvwd> On Tuesday, 3 June 2014, 13:32, Eric Jensen wrote:

>>
>>
>>E.g. here's a comparion between standard wxArrayString and
>>wxArrayString using Windows' string compare function (German locale):
>>>
Cvwd> Do you have link with the reference of Windows' string compare function?

http://msdn.microsoft.com/en-us/library/windows/desktop/dd317759%28v=vs.85%29.aspx

Eric


Frode Roxrud Gill

unread,
Jun 3, 2014, 5:34:56 PM6/3/14
to wx-dev
On 3 June 2014 23:05, Vadim Zeitlin <va...@wxwidgets.org> wrote:
>
> If we can just ensure that "a" compares the same with "AA" under all
> platforms, this would be good enough for me.

Actually, in Norwegian, "a" is the first letter of the alphabeth and
"aa" (as a stand-in for "å") is the last letter of the alphabeth, so
using a Norwegian locale "a" and "AA" should not compare equal at
all...

<URL: http://en.wikipedia.org/wiki/Danish_and_Norwegian_alphabet#History >

--
Frode Roxrud Gill

Vadim Zeitlin

unread,
Jun 3, 2014, 5:40:39 PM6/3/14
to wx-...@googlegroups.com
On Tue, 3 Jun 2014 23:34:35 +0200 Frode Roxrud Gill wrote:

FRG> On 3 June 2014 23:05, Vadim Zeitlin <va...@wxwidgets.org> wrote:
FRG> >
FRG> > If we can just ensure that "a" compares the same with "AA" under all
FRG> > platforms, this would be good enough for me.
FRG>
FRG> Actually, in Norwegian, "a" is the first letter of the alphabeth and
FRG> "aa" (as a stand-in for "å") is the last letter of the alphabeth, so
FRG> using a Norwegian locale "a" and "AA" should not compare equal at
FRG> all...

Just to be clear: by "same" I meant "comparison should give the same
result under all platforms". I probably could have relaxed this even
further and said "under C locale". Because, really, we are not going to be
doing anything about strcmp() or strcasecmp() here, this has nothing to do
with wxUniv at this stage.

Regards,
VZ

Catalin

unread,
Jun 3, 2014, 7:21:53 PM6/3/14
to wx-...@googlegroups.com
On Wednesday, 4 June 2014, 0:05, Vadim Zeitlin wrote:


If we can just ensure that "a" compares the same with "AA" under all
platforms, this would be good enough for me.
Ok, but I can't find the way this comparison works even under MSW. By the words of a person from Microsoft Online Community Support [1], CBS_SORT style uses a sorting "roughly equivalent to CompareString (ignore case)", but it might also use some locale settings.

As long as this UI sorting is not documented somewhere and, more important, not guaranteed to work in the same way under different Windows versions, how should it be implemented for different platforms?


Yet another fun to read post here: http://www.siao2.com/2008/10/29/9022296.aspx

Regards,
C

Vadim Zeitlin

unread,
Jun 3, 2014, 7:29:21 PM6/3/14
to wx-...@googlegroups.com
On Tue, 3 Jun 2014 16:20:16 -0700 (PDT) 'Catalin' via wx-dev wrote:

Cvwd> Ok, but I can't find the way this comparison works even under MSW. By
Cvwd> the words of a person from Microsoft Online Community Support
Cvwd> [1], CBS_SORT style uses a sorting "roughly equivalent to
Cvwd> CompareString (ignore case)", but it might also use some locale
Cvwd> settings.
Cvwd>
Cvwd> As long as this UI sorting is not documented somewhere and, more
Cvwd> important, not guaranteed to work in the same way under different
Cvwd> Windows versions, how should it be implemented for different
Cvwd> platforms?

I think any reasonable implementation would do. I suggested looking at
wxMSW just to choose the behaviour for the strings differing by case only
(we still need to return some deterministic value for them for sort to work
at all) but I honestly didn't want to open this can of worms.

Regards,
VZ

Catalin

unread,
Jun 12, 2014, 7:59:06 AM6/12/14
to wx-...@googlegroups.com
Hello,

I'm trying to find a way to have a custom comparison function for wxSortedArrayString.

A loosely related question: could wxArray be a typedef of wxVector or somehow implemented in terms of it? I'm not sure of the implications, but if there is a remote chance of having this I believe it is worth exploring.

A few things about wxArrayString when having !wxUSE_STD_CONTAINERS:
a custom sorting function is only used in wxArrayString::Sort();
wxArrayString has the following undocumented constructor wxArrayString(int autoSort); this looks like a bad idea, because in case m_autoSort == true, wxArrayString::Add() will always use wxString::Cmp() as the comparison function, so the following scenario looks erroneous:
    create a wxArrayString with autoSort = true;
    add some elements - they will be added according to wxString::Cmp();
    call Sort(some custom comparison function);
    add another element - it is not guaranteed that the element will be placed at the end, nor will it be placed according to the custom comparison function, but to wxString::Cmp();
...which leads to the question: should wxArrayString(int autoSort) be removed and m_autoSort made protected so that it can be set by wxSortedStringArray?

When having wxUSE_STD_CONTAINERS set to true I find it very difficult to follow all the macros used for wxArrayStringBase. Any help with this case would be welcome.

Regards,
C

Catalin

unread,
Jun 12, 2014, 8:05:23 AM6/12/14
to wx-...@googlegroups.com
On Thursday, 12 June 2014, 14:59, 'Catalin' via wx-dev <wx-...@googlegroups.com> wrote:


...which leads to the question: should wxArrayString(int autoSort) be removed and m_autoSort made protected so that it can be set by wxSortedStringArray?
I just realised that m_autoSort does not need to be protected. wxSortedStringArray constructor can call Init(true).
So the only question is whether that constructor should be removed or not.

Regards,
C

Vadim Zeitlin

unread,
Jun 12, 2014, 9:10:22 AM6/12/14
to wx-...@googlegroups.com
On Thu, 12 Jun 2014 04:59:04 -0700 (PDT) 'Catalin' via wx-dev wrote:

Cvwd> A loosely related question: could wxArray be a typedef of wxVector or
Cvwd> somehow implemented in terms of it? I'm not sure of the implications,
Cvwd> but if there is a remote chance of having this I believe it is worth
Cvwd> exploring.

I am not sure how to do it while preserving the compatibility. If it could
be done, it would definitely be great, of course, nobody wants to deal with
the macro horrors in wx/dynarray.h and wx/arrstr.h.

Cvwd> A few things about wxArrayString when having !wxUSE_STD_CONTAINERS:
Cvwd> - a custom sorting function is only used in wxArrayString::Sort();

There are 2 possibilities:

1. Change this and allow specifying a custom sort function, e.g. in
wxSortedArrayString ctor.
2. Don't use wxSortedArrayString when custom sorting is needed.

While (1) could be seen as more generally useful, I think (2) would be
actually simpler... You'd just need to call Sort() with the custom
comparator after modifying the array. Of course, it's much less efficient
than inserting an item into an already sorted array...

Cvwd> - wxArrayString has the following undocumented
Cvwd> constructor wxArrayString(int autoSort); this looks like a bad idea,
Cvwd> because in case m_autoSort == true, wxArrayString::Add() will always
Cvwd> use wxString::Cmp() as the comparison function, so the following
Cvwd> scenario looks erroneous:
Cvwd>     create a wxArrayString with autoSort = true;
Cvwd>     add some elements - they will be added according to wxString::Cmp();
Cvwd>     call Sort(some custom comparison function);

This won't work, Sort() has an assert checking that the array is not
auto-sorted.

Cvwd> ...which leads to the question: should wxArrayString(int autoSort) be
Cvwd> removed and m_autoSort made protected so that it can be set by
Cvwd> wxSortedStringArray?

I don't see any particular problem with removing this ctor but OTOH I
don't see any real reason to remove it neither and, when in doubt, I'd
rather avoid touching this code.

Cvwd> When having wxUSE_STD_CONTAINERS set to true I find it very difficult
Cvwd> to follow all the macros used for wxArrayStringBase. Any help with
Cvwd> this case would be welcome.

I can't say I can follow them easily neither, but it looks that in this
case we already have (1), i.e. the comparison function can be specified as
wxSortedStringArrayBase ctor argument. So if you choose the solution (1),
you almost don't need to do anything in wxUSE_STD_CONTAINERS case. And, of
course, if you choose the solution (2), you don't need to change anything
in this file at all.

Regards,
VZ

Catalin

unread,
Jun 13, 2014, 8:49:28 PM6/13/14
to wx-...@googlegroups.com
On Thursday, 12 June 2014, 16:10, Vadim Zeitlin wrote:


On Thu, 12 Jun 2014 04:59:04 -0700 (PDT) Catalin wrote:

Cvwd> A few things about wxArrayString when having !wxUSE_STD_CONTAINERS:
Cvwd> - a custom sorting function is only used in wxArrayString::Sort();

There are 2 possibilities:

1. Change this and allow specifying a custom sort function, e.g. in
  wxSortedArrayString ctor.
One possibility for 1. could be to have a protected CompareFunction function pointer in wxArrayString, to be used from wxArrayString::Add(), but which will be NULL by default and only set from wxSortedArrayString(CompareFunction f).
When the pointer is NULL it will use the default wxString::Cmp().
2. Don't use wxSortedArrayString when custom sorting is needed.
This would be easy to do in new code. But for already written wxW internals I don't trust myself to track all places where a formerly sorted array is modified and call Sort() on it, rather than give it a custom comparison function when the variable is created and then forget about it.
Cvwd> - wxArrayString has the following undocumented
Cvwd> constructor wxArrayString(int autoSort); this looks like a bad idea,
Cvwd> because in case m_autoSort == true, wxArrayString::Add() will always
Cvwd> use wxString::Cmp() as the comparison function, so the following
Cvwd> scenario looks erroneous:
Cvwd>     create a wxArrayString with autoSort = true;
Cvwd>     add some elements - they will be added according to wxString::Cmp();
Cvwd>     call Sort(some custom comparison function);

This won't work, Sort() has an assert checking that the array is not
auto-sorted.
Right, I missed that. It's ok then.


Regards,
C

Vadim Zeitlin

unread,
Jun 13, 2014, 9:04:46 PM6/13/14
to wx-...@googlegroups.com
On Fri, 13 Jun 2014 17:49:26 -0700 (PDT) 'Catalin' via wx-dev wrote:

Cvwd> >Cvwd> A few things about wxArrayString when having !wxUSE_STD_CONTAINERS:
Cvwd> >Cvwd> - a custom sorting function is only used in wxArrayString::Sort();
Cvwd> >
Cvwd> >There are 2 possibilities:
Cvwd> >
Cvwd> >1. Change this and allow specifying a custom sort function, e.g. in
Cvwd> >  wxSortedArrayString ctor.
Cvwd> One possibility for 1. could be to have a protected CompareFunction function pointer in wxArrayString, to be used from wxArrayString::Add(), but which will be NULL by default and only set from wxSortedArrayString(CompareFunction f).
Cvwd> When the pointer is NULL it will use the default wxString::Cmp().

This should work, yes.

Cvwd> > 2. Don't use wxSortedArrayString when custom sorting is needed.
Cvwd> This would be easy to do in new code. But for already written wxW
Cvwd> internals I don't trust myself to track all places where a formerly
Cvwd> sorted array is modified and call Sort() on it, rather than give it a
Cvwd> custom comparison function when the variable is created and then
Cvwd> forget about it.

I actually think it's not that difficult, there are only a couple of
methods modifying the array and it's only when using Add() and Insert()
that we need to worry about the sort order. The main advantage of this
approach I see is that it would be easier to understand for the people
used to standard, non-sorted, containers.


But, of course, fixing this in some way is still much better than not
fixing it at all, so if you prefer (1), please do it like this by all
means.

TIA,
VZ

Catalin

unread,
Jun 15, 2014, 3:49:27 PM6/15/14
to wx-...@googlegroups.com
> On Saturday, 14 June 2014, 4:04, Vadim Zeitlin wrote:

> > On Fri, 13 Jun 2014 17:49:26 -0700 (PDT) 'Catalin' via wx-dev wrote:
>
> Cvwd> >1. Change this and allow specifying a custom sort function, e.g. in
> Cvwd> >   wxSortedArrayString ctor.

One more problem that I see is that in wxUSE_STD_CONTAINERS=1 build, the function type used for sorting is
CMPFUNCwxString taking (wxString*, wxString*)
while wxArrayString::Sort() is documented to use
CompareFunction taking (const wxString &first, const wxString &second).
The latter is easy to pass to a new wxSortedArrayString ctor in wxUSE_STD_CONTAINERS=0, but not to a wxSortedArrayStringBase ctor in wxUSE_STD_CONTAINERS=1.

Should the implementation of wxSortedArrayStringBase change to use a CompareFunction(const wxString &first, const wxString &second) ?

Regards,
C

Vadim Zeitlin

unread,
Jun 15, 2014, 5:10:04 PM6/15/14
to wx-...@googlegroups.com
On Sun, 15 Jun 2014 12:49:25 -0700 (PDT) 'Catalin' via wx-dev wrote:

Cvwd> > On Saturday, 14 June 2014, 4:04, Vadim Zeitlin wrote:
Cvwd>
Cvwd> > > On Fri, 13 Jun 2014 17:49:26 -0700 (PDT) 'Catalin' via wx-dev wrote:
Cvwd> >
Cvwd> > Cvwd> >1. Change this and allow specifying a custom sort function, e.g. in
Cvwd> > Cvwd> >   wxSortedArrayString ctor.
Cvwd>
Cvwd> One more problem that I see is that in wxUSE_STD_CONTAINERS=1 build, the function type used for sorting is
Cvwd> CMPFUNCwxString taking (wxString*, wxString*)
Cvwd> while wxArrayString::Sort() is documented to use
Cvwd> CompareFunction taking (const wxString &first, const wxString &second).
Cvwd> The latter is easy to pass to a new wxSortedArrayString ctor in wxUSE_STD_CONTAINERS=0, but not to a wxSortedArrayStringBase ctor in wxUSE_STD_CONTAINERS=1.
Cvwd>
Cvwd> Should the implementation of wxSortedArrayStringBase change to use a CompareFunction(const wxString &first, const wxString &second) ?

Yes, I think it would be better. Not only would this be more consistent,
but it just makes sense to use (const) references here, as the values being
compared are never NULL and never modified.

Regards,
VZ
Reply all
Reply to author
Forward
0 new messages