Greg Sheremeta has posted comments on this change.
Change subject: userportal, webadmin: added branding cascading resource servlet
......................................................................
Patch Set 6:
(1 comment)
....................................................
File
backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/branding/BrandingManager.java
Line 292: List<BrandingTheme> brandingThemes = getBrandingThemes();
// assume these are sorted 00, 01, 02, etc.
Line 293: for (int i = brandingThemes.size() - 1; i >= 0; i--) {
Line 294: String resourcePath =
brandingThemes.get(i).getResourcePath(resourceName);
Line 295: if (resourcePath != null) {
Line 296: return resourcePath;
You have valid points, but I just don't agree with you that we should never
return in the middle of a function. I agree with the top two answers posted
here, specifically the second one:
http://stackoverflow.com/questions/36707/should-a-function-have-only-one-return-statement
Clarity is in the eye of the beholder. I think this function is very clear with
the return in the middle. If I don't return, I have to break the loop.
Returning right when I find what I'm looking for is much clearer (IMHO) than
breaking, or setting an intermediate variable, or setting flags to break the
loop.
Anyway, I don't think we need to argue about this. We both have well-formed
opinions. I'm not changing mine. I doubt you'll change yours. Unless there is a
coding standard for the project that specifically says I should not do this,
I'd like to leave the method as-is.
I resent the patch to address all your other concerns. Please re-review. I'm
hoping to get this merged today and I'll need your +2. Thanks!
Line 297: }
Line 298: }
Line 299: }
Line 300: return null;
--
To view, visit http://gerrit.ovirt.org/18385
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I1dcd9d0f22a7c5867c39576020ad16dd6c53deda
Gerrit-PatchSet: 6
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Greg Sheremeta <[email protected]>
Gerrit-Reviewer: Alexander Wels <[email protected]>
Gerrit-Reviewer: Alon Bar-Lev <[email protected]>
Gerrit-Reviewer: Einav Cohen <[email protected]>
Gerrit-Reviewer: Greg Sheremeta <[email protected]>
Gerrit-Reviewer: Vojtech Szocs <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
