Martin Mucha has posted comments on this change.

Change subject: webadmin: removed ugly if-else chain
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.ovirt.org/#/c/33619/1/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/dataprovider/AsyncDataProvider.java
File 
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/dataprovider/AsyncDataProvider.java:

Line 3224:     public Guid getEntityGuid(Object entity) {
Line 3225: 
Line 3226:         if (entity instanceof BusinessEntity) {
Line 3227:             //BusinessEntity can have lot of different ID types, but 
from this context it cannot be determined.
Line 3228:             Object id = getEntityGuid((BusinessEntity<?>) entity);
> Why is this method needed? You're already casting the entity - might as wel
simple. Instanceof operator is shit. It's usage is indication of design 
failure. So if I can make detour over clean lands, it's better. And it's also 
purpose of this method: getEntityGuid is overloaded and 
getEntityGuid(BusinessEntity) will be used if called with specific type instead 
of Object. The benefits are: 1) it works also for non-guid id business entities 
where latter fails, 2) avoiding shitty operators and shitty if-else chains.
Line 3229: 
Line 3230:             //check whether result can be casted to Guid, otherwise 
continue with explicit rules.
Line 3231:             if (id instanceof Guid) {
Line 3232:                 return (Guid) id;


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic716dc67b90c1d575de14c40b504c984bf0467f2
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Martin Mucha <mmu...@redhat.com>
Gerrit-Reviewer: Lior Vernia <lver...@redhat.com>
Gerrit-Reviewer: Martin Mucha <mmu...@redhat.com>
Gerrit-Reviewer: Tomas Jelinek <tjeli...@redhat.com>
Gerrit-Reviewer: automat...@ovirt.org
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