Saggi Mizrahi has posted comments on this change.
Change subject: Refactor UpdateVdsGroupCommand
......................................................................
Patch Set 4:
Returning true or false is inconsequential to the general philosophy of early
return.
You return early, with any value, to denote that you need no further inspection
to reach your conclusion.
lets take this example of trying to calculate a and b are of equal content:
---------------
def equal(a, b):
if id(b) == id(a):
return True
if type(a) != type(b):
return False
for attr in dir(a):
attrA = getattr(a, attr)
attrB = getattr(a, attr)
if not equal(attrA, attrB):
return False
return True
-----------------------
as You can see I have 4 return points. Each return point returns a different
unrelated value. The logic it expresses as followed:
1. I know that if it's the same object it is equal to itself.
2. I know that if they are not of the same type they are not equal.
3. If one of the attributes is different they are not equal. 4. If I have
nothing else to test then the result must be true.
As you can see early return is not bound to true or even just to boolean return
values. This is a way to say, I have made sufficient tests and operations to
reach a conclusion.
I agree that in most cases. When early returning true. It's an optimization.
(If nothing changed no need to recheck). Like most optimizations it might add
some complexity to the code flow. You can remove these conditionals if you
think they are too confusing and fall back to do the full test. Everything
would still work. However, I think that after I split everything up. Each
method is short enough for someone to be able to "decipher" the optimization.
Also, like any optimization, I have to make sure that when the logic changes it
is still valid.
As I said in my previous comment, all logic changes have to be done with full
understanding of the subroutine you are modifying. This should be done to keep
the interface and semantics it represents, early returns or not. For this exact
reason I split everything in the first place. So when I change things I have
less to worry about I only have to understand the subroutine I am modifying.
--
To view, visit http://gerrit.ovirt.org/2271
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I47300fec2f6f5fabdbc75e8c78332c5aef2508a2
Gerrit-PatchSet: 4
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Saggi Mizrahi <[email protected]>
Gerrit-Reviewer: Ayal Baron <[email protected]>
Gerrit-Reviewer: Haim Ateya <[email protected]>
Gerrit-Reviewer: Maor Lipchuk <[email protected]>
Gerrit-Reviewer: Omer Frenkel <[email protected]>
Gerrit-Reviewer: Saggi Mizrahi <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
