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

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.

Garth

> --
> Anders
> 
> _______________________________________________
> 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