Vojtech Szocs has posted comments on this change. Change subject: frontend: refactoring: Generify Events and Listeners ......................................................................
Patch Set 11: (1 comment) Nice patch, just one comment (inline). Maybe it was discussed before, though. http://gerrit.ovirt.org/#/c/32837/11/frontend/webadmin/modules/uicompat/src/main/java/org/ovirt/engine/ui/uicompat/IEventListener.java File frontend/webadmin/modules/uicompat/src/main/java/org/ovirt/engine/ui/uicompat/IEventListener.java: Line 1: package org.ovirt.engine.ui.uicompat; Line 2: Line 3: public interface IEventListener<T extends EventArgs> { Line 4: void eventRaised(Event<? extends T> ev, Object sender, T args); I don't think that "? extends T" is appropriate here, it could be simply: void eventRaised(Event<T> ev, Object sender, T args); eventRaised method is *not* meant to be called by user code, so what's the reason for having "? extends T" here? Personally I think that "? extends T" just pollutes the API, for example this: getPropertyChangedEvent().addListener(new IEventListener<PropertyChangedEventArgs>() { @Override public void eventRaised(Event<? extends PropertyChangedEventArgs> ev, Object sender, PropertyChangedEventArgs args) { ... } }); Is practically the same thing as this: getPropertyChangedEvent().addListener(new IEventListener<PropertyChangedEventArgs>() { @Override public void eventRaised(Event<PropertyChangedEventArgs> ev, Object sender, PropertyChangedEventArgs args) { ... } }); Therefore I see no practical reason for having "? extends T" as part of eventRaised method signature. -- To view, visit http://gerrit.ovirt.org/32837 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Icddf5780c00c985966e6ae956a401c3ede6a68e7 Gerrit-PatchSet: 11 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Martin Betak <[email protected]> Gerrit-Reviewer: Alexander Wels <[email protected]> Gerrit-Reviewer: Daniel Erez <[email protected]> Gerrit-Reviewer: Greg Sheremeta <[email protected]> Gerrit-Reviewer: Lior Vernia <[email protected]> Gerrit-Reviewer: Martin Betak <[email protected]> Gerrit-Reviewer: Tomas Jelinek <[email protected]> Gerrit-Reviewer: Vojtech Szocs <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
