On 08/03/11 23:17, Johan Hake wrote:
On Tuesday March 8 2011 14:47:00 Anders Logg wrote:
On Mon, Mar 07, 2011 at 07:46:54AM +0000, Garth N. Wells wrote:
On 07/03/11 07:40, Marie E. Rognes wrote:
On 03/07/2011 08:37 AM, Garth N. Wells wrote:
On 06/03/11 22:23, Marie E. Rognes wrote:
On 03/06/2011 10:41 PM, Garth N. Wells wrote:
On 06/03/11 21:29, Marie E. Rognes wrote:
On 03/06/2011 10:22 PM, Anders Logg wrote:
On Sun, Mar 06, 2011 at 09:08:10PM +0000, Garth N. Wells wrote:
On 06/03/11 21:02, Marie E. Rognes wrote:
On 03/06/2011 09:30 PM, noreply@xxxxxxxxxxxxx wrote:
if (parameters["max_dimension"].change_count()> 0
&& V.dim()> max_dimension)
+ {
return true;
-
- // Otherwise, not done.
- return false;
+ }
+ else
+ return false;
}
I notice that my early returns keep getting moved into else
clauses... I
find this approach less readable, especially when there are
nested ifs.
Why is it the preferred way?
Because your comment basically says else, so I'd say it's better
to have
the code say it consistently.
I find it easier to follow, because it's clear that the function
exits
from the conditional block. The return value is either true or
false depending on the one true/false evaluation.
The code is an if -- else if -- else. I don't see how moving that
into an if, if -- else increases consistency.
It was an if -- else.
No, it was not. (It was an "done if A", "done if B", otherwise "not
done")
Looks to me like an if - else structure. It was
if (parameters["max_dimension"].change_count()> 0
&& V.dim()> max_dimension)
return true;
// Otherwise, not done.
return false;
Only if you ignore the first if ;-) The original read as:
// Done if error is less than tolerance
if (std::abs(error_estimate)< tolerance)
return true;
// Or done if dimension is larger than max dimension (and that
// parameter is set).
const uint max_dimension = parameters["max_dimension"];
if (parameters["max_dimension"].change_count()> 0
&& V.dim()> max_dimension)
return true;
// Otherwise, not done.
return false;
This for me is now an even better example :) of why we should use
if - else if - else
for these simple cases (in which nothing is done between statements). It
would have much clearer immediately how the return value is being
computed.
I still think the else is unecessary! :-P
For what it is worth I agree with Anders and Marie. I cannot find it right
now, but reducing the amount of indented code using return statements from a
function is considered good programing (by some guidlines at least) as it
makes the code easier to read. I always seek to flesh out the simples options
first and return the values for these from any function.
I don't know what there is to agree with, because for the actual code in
question indentation is *not* an issue. I suspect that Marie and myself
are the only ones that have looked at the code in question. It is:
// Done if error is less than tolerance
if (std::abs(error_estimate)< tolerance)
return true;
// Or done if dimension is larger than max dimension (and that
// parameter is set).
const uint max_dimension = parameters["max_dimension"];
if (parameters["max_dimension"].change_count()> 0
&& V.dim()> max_dimension)
{
return true;
}
// Otherwise, not done.
return false;
My preference is change it to:
if (std::abs(error_estimate)< tolerance)
return true;
else if (parameters["max_dimension"].change_count()> 0
&& V.dim()> max_dimension)
{
return true
}
else
return false;
which I find clearer.
Related to an earlier post, while 'else' can be avoided in the above, I
don't think that it's 'silly' to use a language element that enhances
readability. If a language provides syntax that self-documents code, I
would prefer this over user code comments.
Garth
I see the point of premature return, with all kindoff error that comes with.
But at the end of the day, I think readable and logical built code with small
and consice functions, are the solution to most of these troubles anyway.
Johan
--
Anders
Garth
and I changed it to
if (parameters["max_dimension"].change_count()> 0
&& V.dim()> max_dimension)
{
return true;
}
else
return false;
Garth
The example in question was pretty trivial, and its precise form not
a big deal. However, I think having a common policy would be
beneficial.
_______________________________________________
Mailing list: https://launchpad.net/~dolfin
Post to : dolfin@xxxxxxxxxxxxxxxxxxx
Unsubscribe : https://launchpad.net/~dolfin
More help : https://help.launchpad.net/ListHelp
_______________________________________________
Mailing list: https://launchpad.net/~dolfin
Post to : dolfin@xxxxxxxxxxxxxxxxxxx
Unsubscribe : https://launchpad.net/~dolfin
More help : https://help.launchpad.net/ListHelp
_______________________________________________
Mailing list: https://launchpad.net/~dolfin
Post to : dolfin@xxxxxxxxxxxxxxxxxxx
Unsubscribe : https://launchpad.net/~dolfin
More help : https://help.launchpad.net/ListHelp
_______________________________________________
Mailing list: https://launchpad.net/~dolfin
Post to : dolfin@xxxxxxxxxxxxxxxxxxx
Unsubscribe : https://launchpad.net/~dolfin
More help : https://help.launchpad.net/ListHelp
_______________________________________________
Mailing list: https://launchpad.net/~dolfin
Post to : dolfin@xxxxxxxxxxxxxxxxxxx
Unsubscribe : https://launchpad.net/~dolfin
More help : https://help.launchpad.net/ListHelp
_______________________________________________
Mailing list: https://launchpad.net/~dolfin
Post to : dolfin@xxxxxxxxxxxxxxxxxxx
Unsubscribe : https://launchpad.net/~dolfin
More help : https://help.launchpad.net/ListHelp
