dolfin team mailing list archive

[Johan Hake <hake.dev@xxxxxxxxx>]

On Mar 6, 2011, at 18:01, Anders Logg <logg@xxxxxxxxx> wrote:

> 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.

I can not find it right now but I have seen the example that Marie presented as "good programing practice" for C\C++ because of the simple reason which both Marie and Anders advocate: simpler code cause less mistakes. 

I also think "else is there for a reason" is a silly argument. 

Johan 

> --
> Anders
> 
> _______________________________________________
> Mailing list: https://launchpad.net/~dolfin
> Post to     : dolfin@xxxxxxxxxxxxxxxxxxx
> Unsubscribe : https://launchpad.net/~dolfin
> More help   : https://help.launchpad.net/ListHelp