Maor Lipchuk has posted comments on this change.
Change subject: engine: Add Quota helper class
......................................................................
Patch Set 4: (6 inline comments)
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/QuotaHelper.java
Line 25: private static LogCompat log =
LogFactoryCompat.getLog(QuotaHelper.class);
Will add final declaration to the log.
I wonder, besides the declaration perspective, are there more advantages to use
final (maybe performance wize)?
Line 30: private static QuotaHelper quotaHelper = new QuotaHelper();
I think its more of a subjective perspective if using static or singleton
class,
I know that mockito had some issues with static references using mockstatic,
causing it to get permgen errors when running it (Although singleton also use
static, so I'm not sure there is any difference)
Although I will adapt your advice and change the field to final.
Line 44: Guid returnedQuotaGuid = quotaId;
Yes, I will change it to be used from the validate step in the command,
but still, I think that if I'm already moving the use of this method to the
validate, there is no reason changing the method.
Unless you think other wise...
Line 47: } else {
Could be, I don't mind changing it.
Line 69: log.error("Unlimited Quota cannot be created, Storage pool
is not valid ");
I like more to log an error instead throwing an exception, I believe its more
elegant then throwing an exception, I think throwing an exception might
reflects immaturity of the project.
Regarding the potential NPE, the client which is using that method, should be
taking that in consider.
Line 96: Guid quotaId = Guid.NewGuid();
Sure, why not.
I tend to agree with this approach.
--
To view, visit http://gerrit.ovirt.org/1327
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie67aad0501bbec91e47740e536eb371b62f42d44
Gerrit-PatchSet: 4
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk <[email protected]>
Gerrit-Reviewer: Laszlo Hornyak <[email protected]>
Gerrit-Reviewer: Maor Lipchuk <[email protected]>
Gerrit-Reviewer: Michael Kublin <[email protected]>
Gerrit-Reviewer: Omer Frenkel <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
