Wicket
  1. Wicket
  2. WICKET-3805

Change Component#visitParents to enable visitors of any type

    Details

    • Type: Improvement Improvement
    • Status: Resolved
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 1.5-RC4
    • Fix Version/s: 6.0.0-beta1
    • Component/s: wicket
    • Labels:
      None

      Description

      The generic type of c accepts any type, but the one in IVisitor restrict it to Components.

      Comonent#visitParents(final Class<?> c, final IVisitor<Component, R> visitor)

      1. WICKET-3805.patch
        10 kB
        Martin Grigorov
      2. WICKET-3805.patch
        3 kB
        Pedro Santos

        Activity

        Hide
        Martin Grigorov added a comment -

        Here is a solution which uses ClassVisitFilter to decide whether the parent class (? extends MarkupContainer) is also some other type (an interface).

        Additionally I changed the signature to accept only '? extends MarkupContainer' as first parameter because this is the minimum what a parent class can be.

        Show
        Martin Grigorov added a comment - Here is a solution which uses ClassVisitFilter to decide whether the parent class (? extends MarkupContainer) is also some other type (an interface). Additionally I changed the signature to accept only '? extends MarkupContainer' as first parameter because this is the minimum what a parent class can be.
        Hide
        Emond Papegaaij added a comment -

        I think the correct signature (if Java would have permitted it) should be:
        public final <R,C extends Component & T, T> R visitParents(final Class<T> c, final IVisitor<? super C, R> visitor)

        The problem is that java does not allow intersection types with type parameters. So we have to choose between 'C extends T' or 'C extends Component'. I think C extends T is the best option, but this makes it impossible to use a visitor on Component if T is an interface:
        public final <R, T> R visitParents(final Class<T> c, final IVisitor<? super T, R> visitor)

        Another option is 2 methods, one for interfaces and one for components, so add this one:
        public final <R, T> R visitParentContainers(final Class<?> c, final IVisitor<? super MarkupContainer, R> visitor)

        In either case, it is important to use '? super X', because it allows you to reuse visitors for which C is bound to a superclass (just as with Comparator).

        Show
        Emond Papegaaij added a comment - I think the correct signature (if Java would have permitted it) should be: public final <R,C extends Component & T, T> R visitParents(final Class<T> c, final IVisitor<? super C, R> visitor) The problem is that java does not allow intersection types with type parameters. So we have to choose between 'C extends T' or 'C extends Component'. I think C extends T is the best option, but this makes it impossible to use a visitor on Component if T is an interface: public final <R, T> R visitParents(final Class<T> c, final IVisitor<? super T, R> visitor) Another option is 2 methods, one for interfaces and one for components, so add this one: public final <R, T> R visitParentContainers(final Class<?> c, final IVisitor<? super MarkupContainer, R> visitor) In either case, it is important to use '? super X', because it allows you to reuse visitors for which C is bound to a superclass (just as with Comparator).
        Hide
        Pedro Santos added a comment -

        Right. Anyway I don't know if we can't add such restriction at this point.
        I'm +1 to relax the API now, like MarkupComponent did, and move to a restrictive one in 1.6

        Show
        Pedro Santos added a comment - Right. Anyway I don't know if we can't add such restriction at this point. I'm +1 to relax the API now, like MarkupComponent did, and move to a restrictive one in 1.6
        Hide
        Martin Grigorov added a comment -

        What will happen if you call:
        visitParents(InterfaceX.class, new IVisitor<InterfaceY, Anything> {}) ?

        and there is no relation between InterfaceX with InterfaceY.
        Wicket will search for InterfaceX and if there is such parent then it will try to pass it to the visitor where it will fail with ClassCastException.

        Show
        Martin Grigorov added a comment - What will happen if you call: visitParents(InterfaceX.class, new IVisitor<InterfaceY, Anything> {}) ? and there is no relation between InterfaceX with InterfaceY. Wicket will search for InterfaceX and if there is such parent then it will try to pass it to the visitor where it will fail with ClassCastException.
        Hide
        Pedro Santos added a comment -

        In MarkupContainer#visitChildren we already allow visitors of a different type from the class being visited.
        e.g.
        container.visitChildren(MarkerInterface.class, new IVisitor<Component, Void>(){});

        The reason I would relax this restriction in visitParents also is that we already know that every parent is a Component, and one may want to use its API without any cast.

        Show
        Pedro Santos added a comment - In MarkupContainer#visitChildren we already allow visitors of a different type from the class being visited. e.g. container.visitChildren(MarkerInterface.class, new IVisitor<Component, Void>(){}); The reason I would relax this restriction in visitParents also is that we already know that every parent is a Component, and one may want to use its API without any cast.
        Hide
        Martin Grigorov added a comment -

        I think even the parameter 'c' should be Class<T>, not Class<?>.

        Show
        Martin Grigorov added a comment - I think even the parameter 'c' should be Class<T>, not Class<?>.
        Hide
        Pedro Santos added a comment -

        minor fix in the last patch

        Show
        Pedro Santos added a comment - minor fix in the last patch
        Hide
        Juergen Donnerstag added a comment -

        Mind you providing a patch. Thanks a lot.

        Show
        Juergen Donnerstag added a comment - Mind you providing a patch. Thanks a lot.
        Hide
        Juergen Donnerstag added a comment -

        right

        Show
        Juergen Donnerstag added a comment - right
        Hide
        Pedro Santos added a comment -

        Both are good options. I prefer relax the visitor since one may want to visit parents of some interface.

        Show
        Pedro Santos added a comment - Both are good options. I prefer relax the visitor since one may want to visit parents of some interface.
        Hide
        Juergen Donnerstag added a comment -

        Can parents be anything else than MarkupContainer (Component)? Shouldn't we rather change "c" than relax the visitor?

        Show
        Juergen Donnerstag added a comment - Can parents be anything else than MarkupContainer (Component)? Shouldn't we rather change "c" than relax the visitor?

          People

          • Assignee:
            Martin Grigorov
            Reporter:
            Pedro Santos
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development