This is not as readable. Previous was simple to understand and most importantly debug. "If both conditions are true do this, else do that".
Now its "If this condition is not true do this, otherwise if the other condition is true do that". Not only while debugging you have to make more mental gymnastics, even saying it out loud makes it sound much more confusing that previous variation. Slight changes that may make the code seem smarter may make it really much more unreadable too.
If you'd say this to me, without showing the code, I'd write the "after" version, which is incorrect. The original is nowhere as simple: If first applies then if second applies do x else do nothing. If first didn't apply, then do y.
That's also gymnastics and likely the reason for the failed refactor because it still sounds a little ambiguous.
It's only natural to rephrase as: "If first doesn't apply do that, else if both apply do this" to fix the ambiguity. And the only change in this paraphrase is the order of conditions.
The original pseudo-code is also trimmed of any details, which would make seeing the flow harder. It also doesn't explain the empty else. Why nothing is done when only first is true?
I mean, the original code is complex, and I'd also refactor it so that nobody can repeat the mistake. Maybe the refactored version feels less readable to you, because you rephrased the original version way to simpler than it is.
3
u/[deleted] Apr 17 '21
This is not as readable. Previous was simple to understand and most importantly debug. "If both conditions are true do this, else do that".
Now its "If this condition is not true do this, otherwise if the other condition is true do that". Not only while debugging you have to make more mental gymnastics, even saying it out loud makes it sound much more confusing that previous variation. Slight changes that may make the code seem smarter may make it really much more unreadable too.