[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: RE: Repobrowser - Numeric order filename sorting

From: Stefan Küng <tortoisesvn_at_gmail.com>
Date: 2006-04-11 11:22:54 CEST

On 4/11/06, Gerasimov, Ivan <Ivan.Gerasimov@transas.com> wrote:

> The comparing routine may not be so complicated. The following algorithm may be used: 1) skip same leading chars, 2) if remaining parts begin with digits, compare strings numerically 3) otherwise, compare strings literally.
>
> Below is a sample function.
> It's not perfect, but it works for the most cases.
                                                    ^^^^^^^^
that's not good enough. It must work in all cases. Otherwise we'll get
tons of bug reports like "numeric sorting doesn't work when ..." or
"sorting corrupt if..." or " ...".
Also, you should know that 'numeric sorting' depends on the currently
used locale - it's not done the same way everywhere.

Another thing you might consider:
what if the numbers actually are dates? The user then might want to
sort by date, and the 'numeric sorting' does it wrong because it
doesn't treat them as dates.
So we then again will get bug reports (after all, the sorting doesn't
do what the user expected).

> // returns true, if x < y, false otherwise.
> bool CompareNumericLess(LPCTSTR x_str, LPCTSTR y_str)
> {
> // skip same characters
> while ((*x_str || *y_str) && *x_str == *y_str)

I'm not comfortable with that. Imagine two identical strings "abc" and "abc".
This loop will increment the pointer up to the \0 char terminating the
strings, and then one char more (because the null char still makes the
while clause true).
So after that while loop, the pointers x_str and y_str point *over the string*.

> {
> ++x_str;
> ++y_str;
> }
>
> // parse numeric part of the first arg
> if (IsCharAlphaNumeric(*x_str) && !IsCharAlpha(*x_str))
> {
> UINT x_num = *x_str - '0';
> LPCTSTR x_str_tmp = x_str + 1;
> while (IsCharAlphaNumeric(*x_str_tmp) && !IsCharAlpha(*x_str_tmp))
> {
> x_num = 10 * x_num + *x_str_tmp - '0';
> ++x_str_tmp;
> }
>
> // parse numeric part of the second arg
> if (IsCharAlphaNumeric(*y_str) && !IsCharAlpha(*y_str))
> {
> UINT y_num = *y_str - '0';
> LPCTSTR y_str_tmp = y_str + 1;
> while (IsCharAlphaNumeric(*y_str_tmp) && !IsCharAlpha(*y_str_tmp))
> {
> y_num = 10 * y_num + *y_str_tmp - '0';
> ++y_str_tmp;
> }
>
> // compare numeric parts of the arguments
> return x_num < y_num;
> }

No you successfully handled strings like
v103
v204
v387
But, it still will be wrong for strings like
v1.3.2
v1.7.8
because you stop at the first '.' char.
You cold try using the atol() function, but that will also only work
with one '.' char (or ',', depending on the locale), not two or three.

Implementing a feature just half way through is worse than having no
feature at all - at least without the feature you don't have to deal
with bugreport mails every day.

Stefan

--
       ___
  oo  // \\      "De Chelonian Mobile"
 (_,\/ \_/ \     TortoiseSVN
   \ \_/_\_/>    The coolest Interface to (Sub)Version Control
   /_/   \_\     http://tortoisesvn.tigris.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tortoisesvn.tigris.org
For additional commands, e-mail: dev-help@tortoisesvn.tigris.org
Received on Tue Apr 11 11:23:14 2006

This is an archived mail posted to the TortoiseSVN Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.