4 ms·
If you put names to those functions it instantly feels a lot less like 'the original is definitely preferrable', for example something like: def handleNewE
by another-dave 4y ago
If you put names to those functions it instantly feels a lot less like 'the original is definitely preferrable', for example something like:
def handleNewEmail(isInboxDisplayed, areNotificationsEnabled):
AddToInbox();
if (isInboxDisplayed):
RefreshInboxView();
if (areNotificationsEnabled):
SendSystemNotification();
Not defending needless refactoring in all cases, it's definitely a judgement call.
(Edit: formatting)
- newaccount74 4y agoIn your example the if statements would be needed in any case, since they depend on external state. In that case the refactoring would make sense. I'm talking about the case where the conditionals are only needed because of the refactoring.
- another-dave 4y agoAh I presumed from your example that conditionals were external to the doABC() logic since they were subsequently passed in as params? To be honest, I find it hard to discuss readibility without a real-world example. The algebraic placeholders feel too terse.
- newaccount74 4y agoI'm talking about the case where you have these two functions: def func1(): ... A() B() C() ... def func2(): ... A() B() C() ... Which is then refactored to def func1(): ... ABC() ... def func2(): ... ABC() ... def ABC(): A() B() C() Which is sensible, but then other functions show up: def func3() ... A() C() ... And you refactor to: def func1(): ... ABC(True) ... def func2(): ... ABC(True) ... def func3() ... ABC(False) ... def ABC(doB): A() if (doB): B() C() I'll leave the final example to your imagination, since I've already used so much screen space. I'm currently in the process of undoing a lot of such refactorings in my codebase because it has become unmanageable. I really think repetitive code is much easier to work with than overabstracted code.
- ColonelPhantom 4y agoI think the main problem is lack of expressiveness (what does True mean?). Since your examples seem to be in Python, I would solve it there with named parameters, maybe even giving them default values. That way the code may be abstract, but it is also informative.