Yair Zaslavsky has posted comments on this change.
Change subject: core: Quota refactor - parameters
......................................................................
Patch Set 7: I would prefer that you didn't submit this
(3 inline comments)
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommandBase.java
Line 596: switch (this.getActionType().getQuotaDependency()) {
Line 597: case NONE:
Line 598: return true;
Line 599: case STORAGE:
Line 600: consumptionParameters.addAll(((QuotaStorageDependent)
this).getQuotaStorageConsumptionParameters());
I really don't like these Downcasts, think how to get rid of them (I understand
why you have QutoaStorageDependant interface, have you considered having a
QuotaDependent Generic interface?
Line 601: break;
Line 602: case VDS:
Line 603: consumptionParameters.addAll(((QuotaVdsDependent)
this).getQuotaVdsConsumptionParameters());
Line 604: break;
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/quota/QuotaConsumptionParametersWrapper.java
Line 67: }
Line 68: return list;
Line 69: }
Line 70:
Line 71: public ArrayList<String> getCanDoActionMessages() {
1. Please return List<String>
2. Not sure I understand why we have to getCanDoActionMessages here.
Explanation:
A. canDoAction messages is something which is returned to user, it's not
something that is sent as parameters to command, So maybe your helper classes
should have different name? Maybe QutoaConsumptionCommandHelper?
Line 72: return this.canDoActionMessages;
Line 73: }
Line 74:
Line 75: public void setCanDoActionMessages(ArrayList<String>
canDoActionMessages) {
Line 71: public ArrayList<String> getCanDoActionMessages() {
Line 72: return this.canDoActionMessages;
Line 73: }
Line 74:
Line 75: public void setCanDoActionMessages(ArrayList<String>
canDoActionMessages) {
See above comment about ArrayList and List.
The fact that GWT is problematic about this issue, it doesn't mean all related
Java code is the same ;)
Line 76: this.canDoActionMessages = canDoActionMessages;
Line 77: }
Line 78:
Line 79: public AuditLogableBase getAuditLogable() {
--
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: 7
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: ofri masad <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Doron Fediuck <[email protected]>
Gerrit-Reviewer: Gilad Chaplik <[email protected]>
Gerrit-Reviewer: Laszlo Hornyak <[email protected]>
Gerrit-Reviewer: Michael Kublin <[email protected]>
Gerrit-Reviewer: Sharad Mishra <[email protected]>
Gerrit-Reviewer: Yair Zaslavsky <[email protected]>
Gerrit-Reviewer: ofri masad <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
