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

[NIkolai Marchenko]

Which actually raises the question, can we get Extract Lambda refactoring
?:)

On Fri, Jun 3, 2016 at 4:59 PM, NIkolai Marchenko <enmarantispam at gmail.com>
wrote:

> 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/87dc3970/attachment.html>