dolfin team mailing list archive

["Garth N. Wells" <gnw20@xxxxxxxxx>]

On 07/03/11 07:40, Marie E. Rognes wrote:
> 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;
> 

This for me is now an even better example :) of why we should use

  if - else if - else

for these simple cases (in which nothing is done between statements). It
would have much clearer immediately how the return value is being computed.

Garth

> -- 
> 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
> 
> 
> _______________________________________________
> Mailing list: https://launchpad.net/~dolfin
> Post to     : dolfin@xxxxxxxxxxxxxxxxxxx
> Unsubscribe : https://launchpad.net/~dolfin
> More help   : https://help.launchpad.net/ListHelp