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, 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.  https://review.openstack.org/#/c/139835/  https://review.openstack.org/#/c/139836/  https://review.openstack.org/#/c/140188/ - Brant
_______________________________________________ OpenStack-dev mailing list OpenStackemail@example.com http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev