dolfin team mailing list archive

[Anders Logg <logg@xxxxxxxxx>]

On Sun, Mar 06, 2011 at 09:45:20PM +0000, Garth N. Wells wrote:
>
>
> On 06/03/11 21:39, Anders Logg wrote:
> > 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.
> >
>
> If it's a straight if - else, I have a pretty strong view that 'else'
> should be used. C/C++ don't provide 'else' for nothing.

'else' does have an effect when there is no return inside the 'if'.

This works:

  int a = 1;
  if (...)
    a += 1;
  else
    a += 2;

This doesn't work:

  int a = 1;
  if (...)
    a += 1;
  a += 2;

But consider this:

  if (...)
    return 1;
  else // this line can be remove and has absolutely no effect
    return 2;

> Related, one strict school of thought says that both 1. and 2. are bad
> since a function should only have one exit. We have had memory leaks
> because of premature exits.

I don't think that's a good policy. It would increase the number of
indentations in general.

--
Anders