Yair Zaslavsky has posted comments on this change.

Change subject: core: Quota refactor - parameters
......................................................................


Patch Set 9: (1 inline comment)

....................................................
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommandBase.java
Line 543:         switch (this.getActionType().getQuotaDependency()) {
Line 544:             case NONE:
Line 545:                 return null;
Line 546:             case STORAGE:
Line 547:                 consumptionParameters = ((QuotaStorageDependent) 
this).getQuotaStorageConsumptionParameters();
Regarding downcast and Gilad's previous comment -

1. We suggested that the default dependency will be NONE - but you guys 
disagreed. Had it been none - your suggestion for empty method might have been 
valid.

2. Even if you have to do downcasts, consider to extract this to methods, you 
have repetition at code (even if one liner - i think the code will look nicer).

3. Please add comments explaining your switch -
The reason you have to add this comments, is that if someone reads your code he 
will not a redundancy and ask himself whether the switch is not redundant (
can there be a scneario in which QutoaDependecy is STROAGE and the command will 
not be instanceof QutoateStrorageDepndent? theoretically you could have coded 
these lines using instanceof,  so please explain clearly at code why you chose 
to use both enums and the interface.
Line 548:                 break;
Line 549:             case VDS:
Line 550:                 consumptionParameters = ((QuotaVdsDependent) 
this).getQuotaVdsConsumptionParameters();
Line 551:                 break;


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iebfc85569ba1aa8bd840f7239f83b7f921a4bd8e
Gerrit-PatchSet: 9
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: ofri masad <oma...@redhat.com>
Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com>
Gerrit-Reviewer: Doron Fediuck <dfedi...@redhat.com>
Gerrit-Reviewer: Gilad Chaplik <gchap...@redhat.com>
Gerrit-Reviewer: Laszlo Hornyak <lhorn...@redhat.com>
Gerrit-Reviewer: Michael Kublin <mkub...@redhat.com>
Gerrit-Reviewer: Sharad Mishra <snmis...@linux.vnet.ibm.com>
Gerrit-Reviewer: Yair Zaslavsky <yzasl...@redhat.com>
Gerrit-Reviewer: ofri masad <oma...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to