Wicket
  1. Wicket
  2. WICKET-5348

JavaDoc for IColumn#getSortProperty() in misleading

    Details

      Description

      JavaDoc for getSortProperty() in org.apache.wicket.extensions.markup.html.repeater.data.table.IColumn<T, S> interface states:

      "Returns the name of the property that this header sorts. If null is returned the header will be unsortable."

      According to this JavaDoc I'd expect that it should be String.

      I assume that someone would like to use something other than String. But I think that at least for PropertyColumn it has to be bound to String, so change the:

      public class PropertyColumn<T, S> extends AbstractColumn<T, S> implements IExportableColumn<T, S, Object>

      to:

      public class PropertyColumn<T> extends AbstractColumn<T, String> implements IExportableColumn<T, String, Object>

      because referenced PropertyModel is not parameterized with S.

      I also suggest to add PropertySortParam that extends SortParam<String> to framework.

      And also IPropertyColumn<T> interface that extends IColumn<T, String>.

      @param <T> for IColumn<T, S> is not described too.

        Activity

        Betlista created issue -
        Hide
        Betlista added a comment -

        Maybe I had to split the "bug" to several bugs, but I feel those parts (wrong JavaDoc, PropertyColumn declaration and PropertySortParam) are closely related.

        Show
        Betlista added a comment - Maybe I had to split the "bug" to several bugs, but I feel those parts (wrong JavaDoc, PropertyColumn declaration and PropertySortParam) are closely related.
        Betlista made changes -
        Field Original Value New Value
        Affects Version/s 6.10.0 [ 12324643 ]
        Hide
        Betlista added a comment -
        Show
        Betlista added a comment - It's related to https://issues.apache.org/jira/browse/WICKET-4535
        Betlista made changes -
        Description JavaDoc for getSortProperty() in org.apache.wicket.extensions.markup.html.repeater.data.table.IColumn<T, S> interface states:

        "Returns the name of the property that this header sorts. If null is returned the header will be unsortable."

        According to this JavaDoc I'd expect that it should be String.

        I assume that someone would like to use something other than String. But I think that at least for PropertyColumn it has to be bound to String, so change the:

        public class PropertyColumn<T, S> extends AbstractColumn<T, S> implements IExportableColumn<T, S, Object>

        to:

        public class PropertyColumn<T> extends AbstractColumn<T, String> implements IExportableColumn<T, String, Object>

        because referenced PropertyModel is not parameterized with S.

        I also suggest to add PropertySortParam that extends SortParam<String> to framework.
        JavaDoc for getSortProperty() in org.apache.wicket.extensions.markup.html.repeater.data.table.IColumn<T, S> interface states:

        "Returns the name of the property that this header sorts. If null is returned the header will be unsortable."

        According to this JavaDoc I'd expect that it should be String.

        I assume that someone would like to use something other than String. But I think that at least for PropertyColumn it has to be bound to String, so change the:

        public class PropertyColumn<T, S> extends AbstractColumn<T, S> implements IExportableColumn<T, S, Object>

        to:

        public class PropertyColumn<T> extends AbstractColumn<T, String> implements IExportableColumn<T, String, Object>

        because referenced PropertyModel is not parameterized with S.

        I also suggest to add PropertySortParam that extends SortParam<String> to framework.

        And also IPropertyColumn<T> interface that extends IColumn<T, String>.
        Betlista made changes -
        Description JavaDoc for getSortProperty() in org.apache.wicket.extensions.markup.html.repeater.data.table.IColumn<T, S> interface states:

        "Returns the name of the property that this header sorts. If null is returned the header will be unsortable."

        According to this JavaDoc I'd expect that it should be String.

        I assume that someone would like to use something other than String. But I think that at least for PropertyColumn it has to be bound to String, so change the:

        public class PropertyColumn<T, S> extends AbstractColumn<T, S> implements IExportableColumn<T, S, Object>

        to:

        public class PropertyColumn<T> extends AbstractColumn<T, String> implements IExportableColumn<T, String, Object>

        because referenced PropertyModel is not parameterized with S.

        I also suggest to add PropertySortParam that extends SortParam<String> to framework.

        And also IPropertyColumn<T> interface that extends IColumn<T, String>.
        JavaDoc for getSortProperty() in org.apache.wicket.extensions.markup.html.repeater.data.table.IColumn<T, S> interface states:

        "Returns the name of the property that this header sorts. If null is returned the header will be unsortable."

        According to this JavaDoc I'd expect that it should be String.

        I assume that someone would like to use something other than String. But I think that at least for PropertyColumn it has to be bound to String, so change the:

        public class PropertyColumn<T, S> extends AbstractColumn<T, S> implements IExportableColumn<T, S, Object>

        to:

        public class PropertyColumn<T> extends AbstractColumn<T, String> implements IExportableColumn<T, String, Object>

        because referenced PropertyModel is not parameterized with S.

        I also suggest to add PropertySortParam that extends SortParam<String> to framework.

        And also IPropertyColumn<T> interface that extends IColumn<T, String>.

        @param <T> for IColumn<T, S> is not described too.
        Hide
        Sven Meier added a comment -

        There could be cases where the whole table uses a certain sort identifier:

        DataTable<Foo, SortIdentifier>

        ... and among other column implementations it uses PropertyColumns:

        new PropertyColumn<Foo, SortIdenfier>(Model.of("Bar"), SortIdentifier.BAR, "bar");

        Highly unlikely but possible.

        Show
        Sven Meier added a comment - There could be cases where the whole table uses a certain sort identifier: DataTable<Foo, SortIdentifier> ... and among other column implementations it uses PropertyColumns: new PropertyColumn<Foo, SortIdenfier>(Model.of("Bar"), SortIdentifier.BAR, "bar"); Highly unlikely but possible.
        Hide
        Betlista added a comment - - edited

        I understand that this is some additional sorting parameter type. Now I understand it's meaning, so if I have some time I'll add patch for JavaDoc and later I'll contribute enahncement for IColumn hierarchy. Thanks

        Show
        Betlista added a comment - - edited I understand that this is some additional sorting parameter type. Now I understand it's meaning, so if I have some time I'll add patch for JavaDoc and later I'll contribute enahncement for IColumn hierarchy. Thanks
        Hide
        Martin Grigorov added a comment -

        I've improved a bit the javadoc by removing "the name of" for #getSortProperty() method.
        Please attach a patch with further improvements when you have some. Thanks!

        Show
        Martin Grigorov added a comment - I've improved a bit the javadoc by removing "the name of" for #getSortProperty() method. Please attach a patch with further improvements when you have some. Thanks!
        Martin Grigorov made changes -
        Status Open [ 1 ] Resolved [ 5 ]
        Assignee Martin Grigorov [ mgrigorov ]
        Fix Version/s 7.0.0 [ 12322958 ]
        Fix Version/s 6.11.0 [ 12324874 ]
        Resolution Fixed [ 1 ]
        Transition Time In Source Status Execution Times Last Executer Last Execution Date
        Open Open Resolved Resolved
        3d 22h 22m 1 Martin Grigorov 16/Sep/13 12:54

          People

          • Assignee:
            Martin Grigorov
            Reporter:
            Betlista
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development