... into 1000-ish lines in python and some xml dialects spread over 20ish files that may or may not do the same thing... covered by 5 tests when there are clearly more then 5 initial states driving the original if....<p>I would have left it as the big if.<p>Source: <a href="https://github.com/groupserver/gs.group.member.canpost" rel="nofollow">https://github.com/groupserver/gs.group.member.canpost</a>
That was an exciting ride! First an if statement that was longer than one would want, then he was splitting things off into small bits and a loop to make it look neat and dynamic, and then it <i>exploded into bullshit</i> and stopped looking like Python altogether.<p>Guess it happens.
Wow, everything I hate about "Enterprise Software" in a single if statement. Have you ever noticed there are entire businesses built on fine-grained access control like this, that are utterly pointless bullshit? Probably full of security holes, leaky abstractions, poorly tested and responsible for creating a lot of support cases and manual intervention.<p>Figure out what the trust modality in your application is, and normally it's a simple graph or can be managed with a straight-forward application of world/group/user UNIX-style permissions.
I don't care how "broken" my multi-line if statement is, I would <i>never</i> add Zope as a requirement.<p>Also, singleton (yuck) accessed via a helper function: yuck. Just import it.<p>This starts to make sense where OP professes fondness of XML: we have a Java developer!
That's not a 300 line if statement, it's broken down into many functions.<p>But anyway, I see the world of enterprise Java has reached Python.
He replaced a huge if/else with a rule engine.<p>The idea is sound since that's exactly what rule engines are for but I feel that if you don't also post the final listing of the rules you end up with, you haven't really made the case that you improved the code.
If you want to actually solve this problem without doing the thing in the slideshow or shooting yourself, when you see<p><pre><code> if(self.predicate_function())
foo()
else
bar()
</code></pre>
and self.predicate_function() is only called once in the program (this is the case several times in the slideshow example), try pulling the if statement into the function (and simplify), making a<p><pre><code> self.handle_case_something()
</code></pre>
function instead. If the predicate is called more than once, consider subclassing. You're already doing OO, may as well do it right.
I tried several times to refactor a 10k+ line switch statement which had a particular case that by all logic should be unreachable, but had a comment something like this:<p><pre><code> /* I don't know why, but if you remove this everything breaks */
</code></pre>
I wonder if that is still there, it's been over ten years.
Is the logic equivalent though? In the initial example, there were at least four outcomes (post, notify not a member, notify not a posting member, notify group closed) which seems too many to map on a boolean.
Perhaps a better tool to refactor any such large if-statement: <a href="http://drakon-editor.sourceforge.net/" rel="nofollow">http://drakon-editor.sourceforge.net/</a>
Typical case of code providing policy instead of mechanism, the solution of course being to provide policy over numerous classes, rules engines, etc instead of writing a few methods that provide mechanism.<p>eg. Why is this method concerned with whether the notification is email or something else...<p>Are persons not also the case of a group with one member?<p>It seems less like refactoring and more like hiding complexity in frameworks.
If all you are doing is trying a bunch of cases (or filters) on some data, why not just use all and a list comprehension on functions.<p>all([postAllowFilter() for postAllowFilter in filters])
Then just take a list of all of your functions.<p>Really, there just should have been a restructuring of the initial code flow. It didn't look too messy.
I noticed a variable named retval in a few slides and was irritated for a moment.<p>Maybe because I stopped using variable names like "retval", "data" or even "result" - because they convey so little meaning.