← Back to team overview

dolfin team mailing list archive

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

 

On Sun, Mar 06, 2011 at 10:29:10PM +0100, 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.
>
>
> >
> >I'm not sure we're talking about the same thing here, but I think the
> >following examples are *good* practice:
> >
> >1. Early return to get rid of special cases
> >
> >void foo()
> >{
> >   // Get rid of some case
> >   if (...)
> >     return;
> >
> >   // Get rid of another case
> >   if (...)
> >     return;
> >
> >   // Do main bulk of work here
> >   ...
> >}
> >
> >Alternative using if/else will force main code in one indentation
> >level.
> >
> >2. Fallback return at end of block
> >
> >bool foo()
> >{
> >   // Handle case 1
> >   if (...)
> >   {
> >     ...
> >
> >     return true;
> >   }
> >
> >   // Handle case 2
> >   if (...)
> >   {
> >     ...
> >
> >     return true;
> >   }
> >
> >   // No foos found
> >   return false;
> >}
>
>
> Yep, I prefer 2. to an if -- else if (-- else if) -- else.

It's also logical since the 'else' has no effect. So the policy can be
to remove code that has zero effect.

--
Anders



Follow ups

References