Click
  1. Click
  2. CLK-344

Column sorting in alphabetical order is incorrectly case sensitive

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 1.4
    • Fix Version/s: 1.5 M1, 1.4.1
    • Component/s: core
    • Labels:
      None
    • Environment:
      Windows XP

      Description

      If sorting a column by alphabetical order, the item "DHT" will appear before "Data Services"

        Activity

        Hide
        Bob Schellink added a comment -

        In JDK6 has exposed a neato solution for locale sorting:
        http://java.sun.com/javase/6/docs/api/java/text/Normalizer.html

        For text we can do:
        String normalizedString = Normalizer.normalize(mystring, Normalizer.Form.NFD);

        For those interested there is a good overview here:
        http://weblogs.java.net/blog/joconner/archive/2007/02/normalization_c.html

        Long ago I wrote a alphanumeric comparator that sorts char for char. I added the Normalizer to it as well. Note that as you stated the Normalizer does have quite a performance impact

        When I have some time I will upload my version to clickclick.

        Show
        Bob Schellink added a comment - In JDK6 has exposed a neato solution for locale sorting: http://java.sun.com/javase/6/docs/api/java/text/Normalizer.html For text we can do: String normalizedString = Normalizer.normalize(mystring, Normalizer.Form.NFD); For those interested there is a good overview here: http://weblogs.java.net/blog/joconner/archive/2007/02/normalization_c.html Long ago I wrote a alphanumeric comparator that sorts char for char. I added the Normalizer to it as well. Note that as you stated the Normalizer does have quite a performance impact When I have some time I will upload my version to clickclick.
        Hide
        Malcolm Edgar added a comment -

        Hi Bob, thanks for the review, great thing about open source.

        I have applied your suggestions, and made the class a public inner class.

        Regarding locale bases sorting, hopefully we will hear from the community if this is required. The performance of locale based sorting can be pretty awful in Java, this was my experience about 7 years ago. However these row lists are generally fairly small so it probably would not be a problem.

        regards Malcolm Edgar

        Show
        Malcolm Edgar added a comment - Hi Bob, thanks for the review, great thing about open source. I have applied your suggestions, and made the class a public inner class. Regarding locale bases sorting, hopefully we will hear from the community if this is required. The performance of locale based sorting can be pretty awful in Java, this was my experience about 7 years ago. However these row lists are generally fairly small so it probably would not be a problem. regards Malcolm Edgar
        Hide
        Bob Schellink added a comment -

        Hi Malcolm,

        Nice work done on this. The ColumnComparator's sorting is much better than String's default.

        In the ColumnComparator I notice you have this check:

        if (value1 instanceof Comparable && value2 instanceof Comparable)

        { .... }

        else if (value1 instanceof Boolean && value2 instanceof Boolean) {

        Boolean implements Comparable. I don't think the second explicit check is needed here.

        Also should ColumnComparator be public? Cannot yet see where it would be useful outside of being the default comparator.

        In my experience comparators are pretty specific to the application. So either one would use the default or set a custom comparator specific to that column or table.

        Perhaps ColumnComparator should be package private in the control package? Or a inner class of Table or Column?

        kind regards

        bob

        Show
        Bob Schellink added a comment - Hi Malcolm, Nice work done on this. The ColumnComparator's sorting is much better than String's default. In the ColumnComparator I notice you have this check: if (value1 instanceof Comparable && value2 instanceof Comparable) { .... } else if (value1 instanceof Boolean && value2 instanceof Boolean) { Boolean implements Comparable. I don't think the second explicit check is needed here. Also should ColumnComparator be public? Cannot yet see where it would be useful outside of being the default comparator. In my experience comparators are pretty specific to the application. So either one would use the default or set a custom comparator specific to that column or table. Perhaps ColumnComparator should be package private in the control package? Or a inner class of Table or Column? kind regards bob
        Hide
        Malcolm Edgar added a comment -

        Take #2 - have actually checked in the code this time

        This implementation adds a Comparator property to the Column class. Will gauge feedback to determine whether global Table comparator property is also required.

        regards Malcolm Edgar

        Show
        Malcolm Edgar added a comment - Take #2 - have actually checked in the code this time This implementation adds a Comparator property to the Column class. Will gauge feedback to determine whether global Table comparator property is also required. regards Malcolm Edgar
        Hide
        Bob Schellink added a comment -

        >Table#setComparator() is a little more tricky, is this a map of comparators keyed by column, or does it override column comparators.

        I think Table's comparator should be the global one and Column is more specific.

        Thus Table's comparator can be set as a default, and each Column can override if it needs to.

        table.setComparator(new AppComparator());
        column.setComparator(new SpecialColumnComparator());

        Show
        Bob Schellink added a comment - >Table#setComparator() is a little more tricky, is this a map of comparators keyed by column, or does it override column comparators. I think Table's comparator should be the global one and Column is more specific. Thus Table's comparator can be set as a default, and each Column can override if it needs to. table.setComparator(new AppComparator()); column.setComparator(new SpecialColumnComparator());
        Hide
        Bob Schellink added a comment -

        -------------------------------------------------------------------------
        Check out the new SourceForge.net Marketplace.
        It's the best place to buy or sell services for
        just about anything Open Source.
        http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace

        Show
        Bob Schellink added a comment - ------------------------------------------------------------------------- Check out the new SourceForge.net Marketplace. It's the best place to buy or sell services for just about anything Open Source. http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace
        Hide
        Malcolm Edgar added a comment -

        Bob, that sounds like a good idea. I think the Column#setComparator method would be good, and by default it returns a comparator for the column.

        Table#setComparator() is a little more tricky, is this a map of comparators keyed by column, or does it override column comparators.

        I will reopen this issue and do some more work.

        Regarding i18n, yes I have skipped this issue entirely, but as you say this gives more scope than the current strategy.

        Show
        Malcolm Edgar added a comment - Bob, that sounds like a good idea. I think the Column#setComparator method would be good, and by default it returns a comparator for the column. Table#setComparator() is a little more tricky, is this a map of comparators keyed by column, or does it override column comparators. I will reopen this issue and do some more work. Regarding i18n, yes I have skipped this issue entirely, but as you say this gives more scope than the current strategy.
        Hide
        Bob Schellink added a comment -

        How about having a Table#setComparator and Column#setComparator method?

        Then advanced sorting can be achieved.

        String sorting on alphanumeric characters is not too good. For example given this list: item2, item1, item11.
        After sorting the order would be:
        item1
        item11
        item2

        What is probably needed is:
        item1
        item2
        item11

        It would be hard for Click to solve all cases especially for i18n apps.

        Exposing the setComparator solves this for advanced cases.

        Show
        Bob Schellink added a comment - How about having a Table#setComparator and Column#setComparator method? Then advanced sorting can be achieved. String sorting on alphanumeric characters is not too good. For example given this list: item2, item1, item11. After sorting the order would be: item1 item11 item2 What is probably needed is: item1 item2 item11 It would be hard for Click to solve all cases especially for i18n apps. Exposing the setComparator solves this for advanced cases.
        Hide
        Malcolm Edgar added a comment -

        Hi Bob,

        Could you please backport this fix to Click 1.4.1

        regards Malcolm Edgar

        -------------------------------------------------------------------------
        Check out the new SourceForge.net Marketplace.
        It's the best place to buy or sell services for
        just about anything Open Source.
        http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace
        _______________________________________________
        Click-development mailing list
        Click-development@lists.sourceforge.net
        https://lists.sourceforge.net/lists/listinfo/click-development

        Show
        Malcolm Edgar added a comment - Hi Bob, Could you please backport this fix to Click 1.4.1 regards Malcolm Edgar ------------------------------------------------------------------------- Check out the new SourceForge.net Marketplace. It's the best place to buy or sell services for just about anything Open Source. http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace _______________________________________________ Click-development mailing list Click-development@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/click-development
        Hide
        Malcolm Edgar added a comment -

        Improve string value sorting with new ColumnComparator class. This replaces use of default Java String comparator for string values.

        Show
        Malcolm Edgar added a comment - Improve string value sorting with new ColumnComparator class. This replaces use of default Java String comparator for string values.

          People

          • Assignee:
            Malcolm Edgar
            Reporter:
            Ben Young
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development