[Development] when to Extract Method (was Re: commas in ctor-init-lists)

[NIkolai Marchenko]

Or you could turn the second expression into a lamba for lazy evaluation
and use it like
bool computationNeeded = [](){ return another.such(that, makes,
the.line.too.long)
   || and.then.some.more(); }
if (alreadyComputed || computationNeeded())

On Fri, Jun 3, 2016 at 4:37 PM, Olivier Goffart <olivier at woboq.com> wrote:

> On Freitag, 3. Juni 2016 12:50:08 CEST Edward Welbourne wrote:
> > On Friday 03 June 2016 10:05:52 Edward Welbourne wrote:
> > >>   if (somee.really(long.and.complicated, expression) ||
> > >>   another.such(that, makes, the.line.too.long) ||
> and.then.some.more())
> > Marc Mutz responded:
> > > To be perfectly blunt: such expressions shouldn't be
> > > allowed. Period. Neither with nor without line breaks. Such monsters
> > > should be subjected to Extract Method with extreme prejudice.
> >
> > I'm fascinated - and we're now on a whole new topic - you mean this,
> > even when the relevant combination of expressions is meaningless outside
> > the one context in which it is used: an expression that combines three
> > computed values with a binary operator is so complex it should be turned
> > into a method ?  Even if the three conditions are unrelated ?
> >
> > How about if all three are tested separately ?
> >
> > if (some.really(long.and.complicated, expression)) {
> >     // this one reason for an early return
> >     delete thing;
> >     return;
> > }
> >
> > if (another.such(that, makes, the.line.too.long)) {
> >     // this quite different reason for it
> >     delete thing;
> >     return;
> > }
> >
> > if (and.then.some.more()) {
> >     // a third quite different reasons
> >     delete thing;
> >     return;
> > }
> >
> > I see lots of code like that.  If combining them into
> >
> > if (some.really(long.and.complicated, expression) // this one reason for
> an
> > early return
> >    // this quite different reason for it:
> >    || another.such(that, makes, the.line.too.long)
> >
> >    // a third quite different reasons:
> >    || and.then.some.more()) {
> >
> >    delete thing;
> >    return;
> > }
> >
> > requires Extract Method, I suspect the method name is going to end up
> > being too long to fit on a line, on account of being
> >
> > thisOneReasonOrThatQuiteDifferentReasonOrThatThirdReason(long,
> expression,
> > that, makes, the, and, another, some);
> >
> > with lots of unrelated parameters in order to do its job.  Well, I
> > suppose its name could be shouldReturnEarlyFromOriginalMethod(...)
> > instead; but I conclude that you actually prefer the three separate
> > clauses, albeit in the extracted method, where each returns true, so
> > that the calling code only has to do its tidy-up (and possibly other)
> > code once.
> >
> > So, I'm fascinated: when is Extract Method the right pattern to apply ?
> > I would normally reserve it for cases where it adds semantic clarity (by
> > naming the condition concisely), avoids duplication or modularises an
> > over-long method,
>
> What i'd possibly do:
>
> bool alreadyComputed = some.really(long.and.complicated, expression);
> bool computationNeeded = another.such(that, makes, the.line.too.long)
>    || and.then.some.more();
> if (alreadyComputed || computationNeeded) {
>     delete thing;
>     return;
> }
>
> If the cost of calling the conditions function is neglectible enough that
> it
> can be done even if the first condition would be false.
>
> --
> Olivier
>
> Woboq - Qt services and support - https://woboq.com -
> https://code.woboq.org
> _______________________________________________
> Development mailing list
> Development at qt-project.org
> http://lists.qt-project.org/mailman/listinfo/development
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.qt-project.org/pipermail/development/attachments/20160603/7b6d63cc/attachment.html>