
CVSROOT: /cvs/ppl Module name: ppl Changes by: mundell@cs.unipr.it 2005-10-04 14:35:46
Modified files: src : Polyhedron_public.cc
Log message: Take out two redundant `else' and a redundant `if'. Correct some comments (mostly typos). Clip a trailing space. Take out a \relates comment on a Polyhedron method.
Patches: http://www.cs.unipr.it/cgi-bin/cvsweb.cgi/ppl/src/Polyhedron_public.cc.diff?...

Matthew Mundell wrote:
Log message: Take out two redundant `else' and a redundant `if'.
What do you mean by redundant else? Notice that source code is more than its semantics: the fact that with and without the else the behavior is the same is not a good reason to prefer the version without (or the version with the `else'). If, in addition, you have something of the form
if (a) ... else // comment return ...
then taking out the else without revising the comment can do more harm than good.
More generally, the rationale for such changes is as follows:
- either things are left as they are on the grounds that who wrote the code knew better if the `else' there was making things more readable or not; - or the issue is raised globally, discussed at length, documented in the STANDARDS file and then systematically applied to whatever code.
Ciao,
Roberto

Roberto Bagnara bagnara@cs.unipr.it writes:
Matthew Mundell wrote:
Log message: Take out two redundant `else' and a redundant `if'.
What do you mean by redundant else?
The `else' repeats the information implied by the final return or goto.
Notice that source code is more than its semantics: the fact that with and without the else the behavior is the same is not a good reason to prefer the version without (or the version with the `else').
Adding the else makes it less clear that the body of the "then" branch of the `if' always ends in a return or goto.
If, in addition, you have something of the form
if (a) ... else // comment return ...
Did you mean this?
if (a) return ... else // comment ...
then taking out the else without revising the comment can do more harm than good.
The comment still applies to the code that was in the else branch.
More generally, the rationale for such changes is as follows:
- either things are left as they are on the grounds that who wrote the code knew better if the `else' there was making things more readable or not;
- or the issue is raised globally, discussed at length, documented in the STANDARDS file and then systematically applied to whatever code.
OK, I will resist from changing such code.

Matthew Mundell wrote:
The `else' repeats the information implied by the final return or goto.
But the `else' makes it clearer that we are talking about mutually exclusive conditions. And it is also more robust: what happens if someone changes the "then" branch without noticing that you removed the `else'?
Adding the else makes it less clear that the body of the "then" branch of the `if' always ends in a return or goto.
If, in addition, you have something of the form
if (a) ... else // comment return ...
Did you mean this?
if (a) return ... else // comment ...
No, I meant what I wrote: in the original version, `else' and the comment are part of the same "phrase". If you take out the `else' then the comment assumes an "unconditional flavor" that can only be recovered by noticing that the "then" branch forces a certain control-flow. Taking out the `else' makes two sections of code that could/should be indepedent from one another strongly coupled: if you want to keep correctness you must make sure the "then" branch has the property to go somewhere else at the end.
To come back to the subject: if you only look at semantics, it is true that such elses are redundant; but if you look at the big picture (and this includes readability) things are more complex. Please notice that I am not arguing in favor of using this kind of elses everywhere: I only question that semantic redundancy is a good reason to take a random `else' and remove it. Whence my original question about the meaning of "redundant else". Cheers,
Roberto

Roberto Bagnara bagnara@cs.unipr.it writes:
Matthew Mundell wrote:
The `else' repeats the information implied by the final return or goto.
But the `else' makes it clearer that we are talking about mutually exclusive conditions.
This should be clear enough from the code.
And it is also more robust: what happens
if someone changes the "then" branch without noticing that you removed the `else'?
It's better if the code breaks in this case, as the person will have introduced the fall-through case so will have needed to check what followed the if statement, even if there was an `else'.
Adding the else makes it less clear that the body of the "then" branch of the `if' always ends in a return or goto.
If, in addition, you have something of the form
if (a) ... else // comment return ...
Did you mean this? if (a) return ... else // comment ...
No, I meant what I wrote: in the original version, `else' and the comment are part of the same "phrase".
The question was whether the return was meant for the `then' block, as that would be required to make the `else' redundant. Anyway, what you were saying was clear.
If you take out the
`else' then the comment assumes an "unconditional flavor" that can only be recovered by noticing that the "then" branch forces a certain control-flow. Taking out the `else' makes two sections of code that could/should be indepedent from one another strongly coupled: if you want to keep correctness you must make sure the "then" branch has the property to go somewhere else at the end.
To come back to the subject: if you only look at semantics, it is true that such elses are redundant; but if you look at the big picture (and this includes readability) things are more complex. Please notice that I am not arguing in favor of using this kind of elses everywhere: I only question that semantic redundancy is a good reason to take a random `else' and remove it.
If "to take a random `else'" refers to me making the changes in question then have you looked at the surrounding code? In both cases the bodies of the `then' clauses are very small (2 statements, one of which is tracing). In the first case very similar code 24 lines above has exactly the same structure.
Whence my original question about the meaning
of "redundant else".
participants (3)
-
Matthew Mundell
-
Matthew Mundell
-
Roberto Bagnara