Open Stack

Not too long ago projects added a maximum complexity check to tox.ini, for
example keystone has "max-complexity=24". Seemed like a good idea at the
time, but in a recent attempt to lower the maximum complexity check in
keystone[1][2], I found that the maximum complexity check can actually lead
to less understandable code. This is because the check includes an embedded
function's "complexity" in the function that it's in.

The way I would have lowered the complexity of the function in keystone is
to extract the complex part into a new function. This can make the existing
function much easier to understand for all the reasons that one defines a
function for code. Since this new function is obviously only called from
the function it's currently in, it makes sense to keep the new function
inside the existing function. It's simpler to think about an embedded
function because then you know it's only called from one place. The problem
is, because of the existing complexity check behavior, this doesn't lower
the "complexity" according to the complexity check, so you wind up putting
the function as a new top-level, and now a reader is has to assume that the
function could be called from anywhere and has to be much more cautious
about changes to the function.

Since the complexity check can lead to code that's harder to understand, it
must be considered harmful and should be removed, at least until the
incorrect behavior is corrected.

[1] https://review.openstack.org/#/c/139835/
[2] https://review.openstack.org/#/c/139836/
[3] https://review.openstack.org/#/c/140188/

- Brant
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstack.org/pipermail/openstack-dev/attachments/20141208/2a8af6aa/attachment.html>