[Development] when to Extract Method (was Re: commas in ctor-init-lists)
Edward Welbourne
edward.welbourne at qt.io
Fri Jun 3 14:50:08 CEST 2016
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,
Eddy.
More information about the Development
mailing list