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