Tomas Jelinek has posted comments on this change.

Change subject: frontend: Select an empty profile does not work
......................................................................


Patch Set 4:

(2 comments)

....................................................
File 
frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/editor/BaseListModelSuggestBox.java
Line 159:     public HandlerRegistration addValueChangeHandler(final 
ValueChangeHandler<T> handler) {
Line 160:         return asSuggestBox().addValueChangeHandler(new 
ValueChangeHandler<String>() {
Line 161:             @Override
Line 162:             public void onValueChange(ValueChangeEvent<String> event) 
{
Line 163:                 try {
...considering the bug was not in the base class which caused problem in the 
derived one like in this case.

Look, I understand what you are trying to say. In general I agree, but if you 
start to face cross cutting concerns like here you can either have more logic 
in upper class than is needed for all the children or to have code duplication 
in children. 

None of this solutions is good but the code duplication is evil evil evil while 
more rich upper class is just not good.
Line 164:                     T value = asEntity(event.getValue());
Line 165:                     handler.onValueChange(new 
ValueChangeEvent<T>(value) {});
Line 166:                 } catch (IllegalArgumentException e) {
Line 167:                     // ignore - the user entered an incorrect string. 
Just do not notify the listeners


Line 161:             @Override
Line 162:             public void onValueChange(ValueChangeEvent<String> event) 
{
Line 163:                 try {
Line 164:                     T value = asEntity(event.getValue());
Line 165:                     handler.onValueChange(new 
ValueChangeEvent<T>(value) {});
I could argue with that but let's not bring up old topics and concentrate on 
the current one.
Line 166:                 } catch (IllegalArgumentException e) {
Line 167:                     // ignore - the user entered an incorrect string. 
Just do not notify the listeners
Line 168:                 }
Line 169:             }


-- 
To view, visit http://gerrit.ovirt.org/18791
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I199d4ce14b55b3f552cf6138b7c3c5d21d620ccc
Gerrit-PatchSet: 4
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Tomas Jelinek <tjeli...@redhat.com>
Gerrit-Reviewer: Alona Kaplan <alkap...@redhat.com>
Gerrit-Reviewer: Daniel Erez <de...@redhat.com>
Gerrit-Reviewer: Lior Vernia <lver...@redhat.com>
Gerrit-Reviewer: Tomas Jelinek <tjeli...@redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to