← Back to team overview

dolfin team mailing list archive

  1. dolfin team
  2. Mailing list archive
  3. Message #21844

Re: [Branch ~dolfin-core/dolfin/main] Rev 5731: Remove parts of pointer/reference interface in Form. Prefer shared_ptr interfaces instead.

  Thread Previous • Date Previous • • Thread Next 
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;

--
Marie




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.

--
Marie




_______________________________________________
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

Follow ups

References

  Thread Previous • Date Previous • • Thread Next