On Mon, Mar 07, 2011 at 07:46:54AM +0000, Garth N. Wells wrote:
>
>
> 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, nore...@launchpad.net 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.
I still think the else is unecessary! :-P
--
Anders
> Garth
>
> >>
> >> 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.
> >>>
> >>>
> >>>
> >>>
> >>>
> >>> _______________________________________________
> >>> Mailing list: https://launchpad.net/~dolfin
> >>> Post to : dolfin@lists.launchpad.net
> >>> Unsubscribe : https://launchpad.net/~dolfin
> >>> More help : https://help.launchpad.net/ListHelp
> >>
> >> _______________________________________________
> >> Mailing list: https://launchpad.net/~dolfin
> >> Post to : dolfin@lists.launchpad.net
> >> Unsubscribe : https://launchpad.net/~dolfin
> >> More help : https://help.launchpad.net/ListHelp
> >
> >
> > _______________________________________________
> > Mailing list: https://launchpad.net/~dolfin
> > Post to : dolfin@lists.launchpad.net
> > Unsubscribe : https://launchpad.net/~dolfin
> > More help : https://help.launchpad.net/ListHelp
>
> _______________________________________________
> Mailing list: https://launchpad.net/~dolfin
> Post to : dolfin@lists.launchpad.net
> Unsubscribe : https://launchpad.net/~dolfin
> More help : https://help.launchpad.net/ListHelp
_______________________________________________
Mailing list: https://launchpad.net/~dolfin
Post to : dolfin@lists.launchpad.net
Unsubscribe : https://launchpad.net/~dolfin
More help : https://help.launchpad.net/ListHelp
